Yarn attributed methods can now use consts as their attribute names.

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,14 +30,16 @@ 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.
 - Auto-advancing `LineView`s will no longer attempt to advance dialogue that has been stopped.
 - `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
 

Editor/Analysis/ActionsRegistrationGenerator~/ActionsGenerator.cs

@@ -121,7 +121,7 @@ public void Execute(GeneratorExecutionContext context)
  var actions = new List<YarnAction>();
  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<string> removals = new HashSet<string>();
+ // 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);
  }

Editor/Analysis/Analyser.cs

@@ -320,17 +320,24 @@ private static AttributeListSyntax GeneratedCodeAttributeList
  }
  }
 
- public static IEnumerable<Action> GetActions(CSharpCompilation compilation, Microsoft.CodeAnalysis.SyntaxTree tree)
+ public static IEnumerable<Action> 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<Action> GetRuntimeDefinedActions(CompilationUnitSyntax root, SemanticModel model)
@@ -390,7 +397,7 @@ private static IEnumerable<Action> GetRuntimeDefinedActions(CompilationUnitSynta
  }
  }
 
- private static IEnumerable<Action> GetAttributeActions(CompilationUnitSyntax root, SemanticModel model)
+ private static IEnumerable<Action> GetAttributeActions(CompilationUnitSyntax root, SemanticModel model, Yarn.Unity.ILogger logger)
  {
  var methodInfos = root
  .DescendantNodes()
@@ -417,21 +424,29 @@ private static IEnumerable<Action> 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();