Skip to content

Commit

Permalink
inaccessible yarn action methods now log they cannot be linked.
Browse files Browse the repository at this point in the history
Previously they were just silently dropped from the code gen, so you weren't aware it wasn't linked to your yarn.
Now it will generate a warning that Unity will show.
Fixes #263
  • Loading branch information
McJones committed Jan 19, 2024
1 parent 4528ee9 commit eb5e40c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 8 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- automatically linking YarnCommand and YarnFunction attributed methods to the dialogue runner
- enabling/disabling C# linking will force an entire C# reimport
- enabling/disabling asset linking will force a reimport of all `yarnprojects`
- `Yarn.Unity.ActionAnalyser.Action` now has a `MethodIdentifierName` property, which is the short form of the method name.

### Changed

Expand All @@ -27,6 +28,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Standard library functions (like `random`, `dice`, `round_places`, etc) have been moved to the core Yarn Spinner library.
- Fixed a bug where the audio assets in the samples weren't being linked correctly resulting in playback errors.
- Intro Sample: Moved the Character Color view to a new stand-alone object (it's easier to explain how to do this in a tutorial!)
- `Analyser` no longer ignores non-public
- this is now handled at the codegen side so we can better log it
- `ActionsGenerator` will now generate C# warnings for non-private methods that are attributed as `YarnFunction` or `YarnCommand`.
- `ActionsGenerator` still logs to a temporary location, but now into a `dev.yarnspinner.logs` folder inside the temporary location.

### Removed

Expand Down
5 changes: 5 additions & 0 deletions Editor/Analysis/Action.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ public class Action
/// </summary>
public string MethodName { get; internal set; }

/// <summary>
/// Gets the short form of the method, essentially the easy to read form of <see cref="MethodName"/>.
/// </summary>
public string MethodIdentifierName { get; internal set; }

/// <summary>
/// Whether this action is a static method, or an instance method.
/// </summary>
Expand Down
45 changes: 42 additions & 3 deletions Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ public void Execute(GeneratorExecutionContext context)
actions.AddRange(Analyser.GetActions(compilation, tree).Where(a => a.DeclarationType == DeclarationType.Attribute));
}

if (actions.Any() == false) {
if (actions.Any() == false)
{
output.WriteLine($"Didn't find any Yarn Actions in {context.Compilation.AssemblyName}. Not generating any source code for it.");
return;
}
Expand All @@ -136,9 +137,34 @@ public void Execute(GeneratorExecutionContext context)
output.WriteLine($"Action is null??");
continue;
}

// if an action isn't public we will log a warning
// and then later we will also skip it
if (action.MethodSymbol.DeclaredAccessibility != Accessibility.Public)
{
var descriptor = new DiagnosticDescriptor(
"YS1001",
$"Yarn {action.Type} methods must be public",
"YarnCommand and YarnFunction methods must be public. \"{0}\" is {1}.",
"Yarn Spinner",
DiagnosticSeverity.Warning,
true,
"[YarnCommand] and [YarnFunction] attributed methods must be public so that the codegen can reference them.",
"https://docs.yarnspinner.dev/using-yarnspinner-with-unity/creating-commands-functions");
context.ReportDiagnostic(Microsoft.CodeAnalysis.Diagnostic.Create(
descriptor,
action.Declaration?.GetLocation(),
action.MethodIdentifierName, action.MethodSymbol.DeclaredAccessibility
));
output.WriteLine($"Action {action.Name} will be skipped due to it's declared accessibility {action.MethodSymbol.DeclaredAccessibility}");
continue;
}
output.WriteLine($"Action {action.Name}: {action.SourceFileName}:{action.Declaration?.GetLocation()?.GetLineSpan().StartLinePosition.Line} ({action.Type})");
}

// removing any actions that aren't public from the list before we do codegen
actions = actions.Where(a => a.MethodSymbol.DeclaredAccessibility == Accessibility.Public).ToList();

output.Write($"Generating source code... ");

var source = Analyser.GenerateRegistrationFileSource(actions);
Expand Down Expand Up @@ -371,10 +397,23 @@ public Yarn.Unity.ILogger GetOutput(GeneratorExecutionContext context)
{
if (GetShouldLogToFile(context))
{
var tempPath = System.IO.Path.GetTempPath();
var tempPath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "dev.yarnspinner.logs");

var path = System.IO.Path.Combine(tempPath, $"{nameof(ActionRegistrationSourceGenerator)}-{context.Compilation.AssemblyName}.txt");
// we need to make the logs folder, but this can potentially fail
// if it does fail then we will just chuck the logs inside the tmp folder
try
{
if (!Directory.Exists(tempPath))
{
Directory.CreateDirectory(tempPath);
}
}
catch
{
tempPath = System.IO.Path.GetTempPath();
}

var path = System.IO.Path.Combine(tempPath, $"{nameof(ActionRegistrationSourceGenerator)}-{context.Compilation.AssemblyName}.txt");
var outFile = System.IO.File.Open(path, System.IO.FileMode.Create);

return new Yarn.Unity.FileLogger(new System.IO.StreamWriter(outFile));
Expand Down
7 changes: 2 additions & 5 deletions Editor/Analysis/Analyser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,6 @@ private static IEnumerable<Action> GetRuntimeDefinedActions(CompilationUnitSynta

private static IEnumerable<Action> GetAttributeActions(CompilationUnitSyntax root, SemanticModel model)
{

var methodInfos = root
.DescendantNodes()
.OfType<MethodDeclarationSyntax>()
Expand All @@ -403,8 +402,7 @@ private static IEnumerable<Action> GetAttributeActions(CompilationUnitSyntax roo
{
return (MethodDeclaration: decl, Symbol: model.GetDeclaredSymbol(decl));
})
.Where(pair => pair.Symbol != null)
.Where(pair => pair.Symbol.DeclaredAccessibility == Accessibility.Public);
.Where(pair => pair.Symbol != null);

var actionMethods = methodsAndSymbols
.Select(pair =>
Expand Down Expand Up @@ -448,6 +446,7 @@ private static IEnumerable<Action> GetAttributeActions(CompilationUnitSyntax roo
Name = actionName,
Type = methodInfo.ActionType,
MethodName = $"{containerName}.{methodInfo.MethodDeclaration.Identifier}",
MethodIdentifierName = methodInfo.MethodDeclaration.Identifier.ToString(),
MethodSymbol = methodInfo.Symbol,
IsStatic = methodInfo.Symbol.IsStatic,
Declaration = methodInfo.MethodDeclaration,
Expand Down Expand Up @@ -492,8 +491,6 @@ private static AsyncType GetAsyncType(IMethodSymbol symbol)
// default value; other parts of the action detection process will throw
// errors.
return default;


}

private static IEnumerable<Parameter> GetParameters(IMethodSymbol symbol)
Expand Down
Binary file modified SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll
Binary file not shown.

0 comments on commit eb5e40c

Please sign in to comment.