diff --git a/CHANGELOG.md b/CHANGELOG.md index aa12074e..9bd4b6e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - 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. - `LineView` now will add in line breaks when it encounters a self closing `[br /]` marker. +- Yarn attributed Functions and Commands can now use constant values in addition to literals for their name. ### Changed @@ -29,7 +30,7 @@ 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 +- `Analyser` no longer ignores non-public Yarn attributed methods - 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. @@ -37,6 +38,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `DialogueAdvanceInput` now supports Virtual Button names in addition to KeyCodes and Input Actions - this can be configured to work on button or keycode release or press. - defaults to on release. +- Actions Registration now dumps generated code into the same temporary folder the logs live in +- `ActionsGenerator` will now generate C# warnings for incorrectly named methods that are attributed as `YarnFunction` or `YarnCommand`. ### Removed diff --git a/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs b/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs index 94e56cc3..79cb5a53 100644 --- a/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs +++ b/Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs @@ -121,7 +121,7 @@ public void Execute(GeneratorExecutionContext context) var actions = new List(); foreach (var tree in compilation.SyntaxTrees) { - actions.AddRange(Analyser.GetActions(compilation, tree).Where(a => a.DeclarationType == DeclarationType.Attribute)); + actions.AddRange(Analyser.GetActions(compilation, tree, output).Where(a => a.DeclarationType == DeclarationType.Attribute)); } if (actions.Any() == false) @@ -130,6 +130,8 @@ public void Execute(GeneratorExecutionContext context) return; } + HashSet removals = new HashSet(); + // validating and logging all the actions foreach (var action in actions) { if (action == null) @@ -157,15 +159,40 @@ public void Execute(GeneratorExecutionContext context) action.MethodIdentifierName, action.MethodSymbol.DeclaredAccessibility )); output.WriteLine($"Action {action.Name} will be skipped due to it's declared accessibility {action.MethodSymbol.DeclaredAccessibility}"); + removals.Add(action.Name); continue; } + + // this is not a full validation of the naming rules of commands + // but is good enough to catch the most common situations, whitespace and periods + if (action.Name.Contains(".") || action.Name.Any(x => Char.IsWhiteSpace(x))) + { + var descriptor = new DiagnosticDescriptor( + "YS1002", + $"Yarn {action.Type} methods must have a valid name", + "YarnCommand and YarnFunction methods follow existing ID rules for Yarn. \"{0}\" is invalid.", + "Yarn Spinner", + DiagnosticSeverity.Warning, + true, + "[YarnCommand] and [YarnFunction] attributed methods must follow Yarn ID rules so that Yarn scripts 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.Name + )); + output.WriteLine($"Action {action.MethodIdentifierName} will be skipped due to it's name {action.Name}"); + removals.Add(action.Name); + 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(); + // removing any actions that failed validation + actions.Where(x => !removals.Contains(x.Name)).ToList(); - output.Write($"Generating source code... "); + output.Write($"Generating source code..."); var source = Analyser.GenerateRegistrationFileSource(actions); @@ -393,25 +420,31 @@ public void Initialize(GeneratorInitializationContext context) context.RegisterForSyntaxNotifications(() => new ClassDeclarationSyntaxReceiver()); } - public Yarn.Unity.ILogger GetOutput(GeneratorExecutionContext context) + static string TemporaryPath() { - if (GetShouldLogToFile(context)) - { - var tempPath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "dev.yarnspinner.logs"); + var tempPath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "dev.yarnspinner.logs"); - // 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 + // 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)) { - tempPath = System.IO.Path.GetTempPath(); + Directory.CreateDirectory(tempPath); } + } + catch + { + tempPath = System.IO.Path.GetTempPath(); + } + return tempPath; + } + + public Yarn.Unity.ILogger GetOutput(GeneratorExecutionContext context) + { + if (GetShouldLogToFile(context)) + { + var tempPath = ActionRegistrationSourceGenerator.TemporaryPath(); var path = System.IO.Path.Combine(tempPath, $"{nameof(ActionRegistrationSourceGenerator)}-{context.Compilation.AssemblyName}.txt"); var outFile = System.IO.File.Open(path, System.IO.FileMode.Create); @@ -431,8 +464,9 @@ private static bool GetShouldLogToFile(GeneratorExecutionContext context) public void DumpGeneratedFile(GeneratorExecutionContext context, string text) { - if (GetShouldLogToFile(context)) { - var tempPath = System.IO.Path.GetTempPath(); + if (GetShouldLogToFile(context)) + { + var tempPath = ActionRegistrationSourceGenerator.TemporaryPath(); var path = System.IO.Path.Combine(tempPath, $"{nameof(ActionRegistrationSourceGenerator)}-{context.Compilation.AssemblyName}.cs"); System.IO.File.WriteAllText(path, text); } diff --git a/Editor/Analysis/Analyser.cs b/Editor/Analysis/Analyser.cs index 506b6cfb..51f2d07a 100644 --- a/Editor/Analysis/Analyser.cs +++ b/Editor/Analysis/Analyser.cs @@ -320,17 +320,24 @@ private static AttributeListSyntax GeneratedCodeAttributeList } } - public static IEnumerable GetActions(CSharpCompilation compilation, Microsoft.CodeAnalysis.SyntaxTree tree) + public static IEnumerable GetActions(CSharpCompilation compilation, Microsoft.CodeAnalysis.SyntaxTree tree, Yarn.Unity.ILogger yLogger = null) { + var logger = yLogger; + if (logger == null) + { + logger = new NullLogger(); + } + var root = tree.GetCompilationUnitRoot(); SemanticModel model = null; - if (compilation != null) { + if (compilation != null) + { model = compilation.GetSemanticModel(tree); } - return GetAttributeActions(root, model).Concat(GetRuntimeDefinedActions(root, model)); + return GetAttributeActions(root, model, logger).Concat(GetRuntimeDefinedActions(root, model)); } private static IEnumerable GetRuntimeDefinedActions(CompilationUnitSyntax root, SemanticModel model) @@ -390,7 +397,7 @@ private static IEnumerable GetRuntimeDefinedActions(CompilationUnitSynta } } - private static IEnumerable GetAttributeActions(CompilationUnitSyntax root, SemanticModel model) + private static IEnumerable GetAttributeActions(CompilationUnitSyntax root, SemanticModel model, Yarn.Unity.ILogger logger) { var methodInfos = root .DescendantNodes() @@ -417,21 +424,29 @@ private static IEnumerable GetAttributeActions(CompilationUnitSyntax roo { var attr = methodInfo.ActionAttribute; - string actionName; + // working on an assumption that most people just use the method name + string actionName = methodInfo.MethodDeclaration.Identifier.ToString(); - // If the attribute has an argument list, and the first argument - // is a string, then use that string's value as the action name. - if (attr.ArgumentList != null - && attr.ArgumentList.Arguments.First().Expression is LiteralExpressionSyntax commandName - && commandName.Kind() == SyntaxKind.StringLiteralExpression - && commandName.Token.Value is string name) - { - actionName = name; - } - else + // handling the situation where they have provided arguments + // if we have an argument list + if (attr.ArgumentList != null) { - // Otherwise, use the method's name. - actionName = methodInfo.MethodDeclaration.Identifier.ToString(); + // we resolve the value of first item in that list + // and if it's a string we use that as the action name + var constantValue = model.GetConstantValue(attr.ArgumentList.Arguments.First().Expression); + if (constantValue.HasValue) + { + if (constantValue.Value is string) + { + logger.WriteLine($"resolved constant expression value for the action name: {constantValue.Value.ToString()}"); + actionName = constantValue.Value as string; + } + else + { + // Otherwise just logging the incorrect type and moving on with our life + logger.WriteLine($"resolved constant expression value for the action name, but it is not a string, skipping: {constantValue.Value.ToString()}"); + } + } } var position = methodInfo.MethodDeclaration.GetLocation(); diff --git a/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll b/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll index 766db4b7..b5037a97 100644 Binary files a/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll and b/SourceGenerator/YarnSpinner.Unity.SourceCodeGenerator.dll differ