From 972848cb5ff3f4b92c90089a9dbbc769c3d77b31 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Sat, 10 Oct 2026 14:02:10 +0200 Subject: [PATCH] VerseGrammar: Open refuses a role named actor and two verbs with one name; edge and command-update pins ValidateAll (one owner, called by AetheriaStores.Open) refuses duplicate verb names; Validate refuses the reserved role name actor. Tests pin the name-class edges (a z 0 9 _), a null role element, the starter verb replaced when only its description or render path differs, the grammar global updated in place, and the fixture's second allegiance and zero-role speak. The test's scoped registry is TestCatalog.Registry(). Builds; unverified until the full run. Co-Authored-By: Claude Opus 5.5 --- Assets/Scripts/ServerShared/AetheriaStores.cs | 2 +- Assets/Scripts/ServerShared/VerseGrammar.cs | 17 ++++ .../VerseGrammarTests.cs | 84 +++++++++++++++++-- 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/Assets/Scripts/ServerShared/AetheriaStores.cs b/Assets/Scripts/ServerShared/AetheriaStores.cs index e1a5ae963..05122fc86 100644 --- a/Assets/Scripts/ServerShared/AetheriaStores.cs +++ b/Assets/Scripts/ServerShared/AetheriaStores.cs @@ -40,7 +40,7 @@ public static class AetheriaStores // cut.md), stat modifiers, role usage (Cut 7). Fails loudly, naming the item. foreach (var data in cache.GetAll()) CultRecordRefs.Validate(data); foreach (var data in cache.GetAll()) CultRecordRefs.Validate(data); - foreach (var verb in cache.GetAll()) VerseGrammarValidation.Validate(verb); + VerseGrammarValidation.ValidateAll(cache.GetAll()); return cache; } catch diff --git a/Assets/Scripts/ServerShared/VerseGrammar.cs b/Assets/Scripts/ServerShared/VerseGrammar.cs index aeb3104fb..00e24833d 100644 --- a/Assets/Scripts/ServerShared/VerseGrammar.cs +++ b/Assets/Scripts/ServerShared/VerseGrammar.cs @@ -57,6 +57,9 @@ public static class VerseGrammarValidation public const int VerbNameMaxLength = 48; public const int RoleNameMaxLength = 32; + // The invoking subject, never a role (Ghostlight refuses a world affordance that names it: ReservedRole). + public const string ActorRole = "actor"; + // Canonical: a lower-case letter, then lower-case letters, digits and underscores, at most maxLength characters. private static bool Canonical(string name, int maxLength) { @@ -67,6 +70,18 @@ public static class VerseGrammarValidation return true; } + // Every verb valid and no two sharing a name: a verb is found by name (Ghostlight refuses DuplicateAffordanceKind). + public static void ValidateAll(IEnumerable verbs) + { + var seen = new HashSet(StringComparer.Ordinal); + foreach (var verb in verbs) + { + Validate(verb); + if (!seen.Add(verb.Name)) + throw new InvalidOperationException($"VerseVerb \"{verb.Name}\": duplicate verb name."); + } + } + // Canonical names, unique role names, defined enum values. A failure names the field and the rule, and the verb once // its name is known canonical; it never prints a value the author wrote that is itself the fault. public static void Validate(VerseVerb verb) @@ -82,6 +97,8 @@ public static class VerseGrammarValidation { if (role == null || !Canonical(role.Name, RoleNameMaxLength)) throw new InvalidOperationException($"VerseVerb \"{verb.Name}\" Roles.Name: not a canonical role name ([a-z][a-z0-9_], 1 to {RoleNameMaxLength} characters)."); + if (role.Name == ActorRole) + throw new InvalidOperationException($"VerseVerb \"{verb.Name}\" Roles.Name: reserved for the invoking subject, never a role."); if (!seen.Add(role.Name)) throw new InvalidOperationException($"VerseVerb \"{verb.Name}\" Roles.Name: duplicate role name."); if (!Enum.IsDefined(typeof(VerseReferentKind), role.Binds)) diff --git a/tests/Aetheria.Shared.Tests/VerseGrammarTests.cs b/tests/Aetheria.Shared.Tests/VerseGrammarTests.cs index 9103299b2..93f2bf0ff 100644 --- a/tests/Aetheria.Shared.Tests/VerseGrammarTests.cs +++ b/tests/Aetheria.Shared.Tests/VerseGrammarTests.cs @@ -49,10 +49,7 @@ public sealed class VerseGrammarTests : IDisposable private static CultCache OpenScoped(string path, bool readOnly) { - var registry = CultDocumentRegistry.ForTypes(typeof(ItemData).Assembly.GetTypes() - .Where(t => t is { IsAbstract: false, IsInterface: false }) - .Where(t => t.GetCustomAttribute() != null)); - var cache = new CultCache(registry); + var cache = new CultCache(TestCatalog.Registry()); cache.AddBackingStore(new SingleFileMessagePackBackingStore(path, readOnly), AetheriaStores.CatalogTypes); return cache; } @@ -105,6 +102,10 @@ public sealed class VerseGrammarTests : IDisposable new object[] { "duplicate role name", (Action)(v => v.Roles = new[] { Good("x").Roles[0], Good("x").Roles[0] }), "duplicate role name", null }, new object[] { "role name with a capital", (Action)(v => v.Roles[0].Name = "Site"), "Roles.Name", "Site" }, new object[] { "role name one past 32 characters", (Action)(v => v.Roles[0].Name = "r" + new string('s', 32)), "Roles.Name", new string('s', 32) }, + new object[] { "a role named for the actor", (Action)(v => v.Roles[1].Name = "actor"), "reserved", null }, + new object[] { "a null role element", (Action)(v => v.Roles = new VerseRole[] { null }), "Roles.Name", null }, + new object[] { "name starting with an underscore", (Action)(v => v.Name = "_haul"), "VerseVerb.Name", "_haul" }, + new object[] { "role name starting with a digit", (Action)(v => v.Roles[0].Name = "0site"), "Roles.Name", "0site" }, new object[] { "referent kind zero", (Action)(v => v.Roles[0].Binds = 0), "Roles.Binds", null }, new object[] { "undefined referent kind", (Action)(v => v.Roles[0].Binds = (VerseReferentKind)99), "Roles.Binds", null }, }; @@ -138,6 +139,35 @@ public sealed class VerseGrammarTests : IDisposable Assert.Contains("VerseVerb.Name", Assert.Throws(() => VerseGrammarValidation.Validate(verb)).Message); } + // The class edges: a and z, 0 and 9, and the underscore, in the first, middle and last position of a verb and a role. + [Theory] + [InlineData("z")] + [InlineData("a")] + [InlineData("zz")] + [InlineData("a0")] + [InlineData("a9")] + [InlineData("a_")] + [InlineData("a_b")] + [InlineData("z0z9_az")] + public void Names_at_the_edges_of_the_canonical_class_are_accepted(string name) + { + var verb = Good(name); + verb.Roles[0].Name = name; + var path = Author("edges-" + name, new VerseGrammar { Revision = 1 }, verb); + using var cache = AetheriaStores.Open(path); + Assert.Equal(name, cache.GetAll().Single().Name); + } + + // A verb is found by name: two with one name are refused wherever they sit. + [Fact] + public void Open_refuses_two_verbs_with_one_name() + { + var path = Author("twins", new VerseGrammar { Revision = 1 }, Good("alpha"), Good("twin"), Good("beta"), Good("twin")); + var error = Assert.Throws(() => AetheriaStores.Open(path)); + Assert.Contains("duplicate verb name", error.Message); + Assert.Contains("\"twin\"", error.Message); + } + [Fact] public void The_longest_canonical_names_are_accepted() { @@ -153,7 +183,10 @@ public sealed class VerseGrammarTests : IDisposable // A scratch root holding a copy of the shipped catalog with its verse grammar removed: the catalog as it was before the // command ran. The catalog is stripped and read through a registry scoped to the shipped assembly, as // PiratesFactionCommandTests does. - private string RootWithoutGrammar(params VerseVerb[] preexisting) + private string RootWithoutGrammar(params VerseVerb[] preexisting) => RootWith(null, preexisting); + + // The same, holding `grammar` (when given) and the preexisting verbs. + private string RootWith(VerseGrammar grammar, params VerseVerb[] preexisting) { var gameData = Path.Combine(_root, "cmd", "GameData"); Directory.CreateDirectory(gameData); @@ -163,6 +196,7 @@ public sealed class VerseGrammarTests : IDisposable { foreach (var grammar in cache.GetAll().ToArray()) Assert.True(cache.Remove(cache.RefOf(grammar).Key)); foreach (var verb in cache.GetAll().ToArray()) Assert.True(cache.Remove(cache.RefOf(verb).Key)); + if (grammar != null) cache.Upsert(grammar); foreach (var verb in preexisting) cache.Upsert(verb); cache.FlushAsync().Wait(); } @@ -229,6 +263,39 @@ public sealed class VerseGrammarTests : IDisposable Assert.Equal(1, cache.GetAll().Count(verb => verb.Name == "mine")); } + // A starter verb that differs in only its description, or only its render path, is replaced too (one record each). + [Theory] + [InlineData("description")] + [InlineData("render path")] + public void Apply_replaces_a_starter_verb_that_differs_in_one_field(string field) + { + var starter = VerseGrammarCommand.StarterVerbs().Single(v => v.Name == "haul"); + var stale = new VerseVerb { Name = starter.Name, RenderPath = starter.RenderPath, Description = starter.Description, Roles = starter.Roles }; + if (field == "description") stale.Description = "An older description."; + else stale.RenderPath = VerseRenderPath.Conversation; + var root = RootWithoutGrammar(stale); + Assert.Equal(0, VerseGrammarCommand.Run(apply: true, root: root)); + + using var cache = OpenScoped(CatalogOf(root), true); + Assert.Equal(1, cache.GetAll().Count(verb => verb.Name == "haul")); + Assert.True(VerseGrammarCommand.Same(starter, cache.GetByName("haul"))); + } + + // The grammar global is updated in place (one record) when its revision or its description is not the starter's. + [Theory] + [InlineData(0, VerseGrammarCommand.StarterDescription)] + [InlineData(VerseGrammarCommand.StarterRevision, "An older description.")] + public void Apply_updates_a_grammar_global_that_is_not_the_starter_in_place(int revision, string description) + { + var root = RootWith(new VerseGrammar { Revision = revision, Description = description }); + Assert.Equal(0, VerseGrammarCommand.Run(apply: true, root: root)); + + using var cache = OpenScoped(CatalogOf(root), true); + var grammar = Assert.Single(cache.GetAll()); + Assert.Equal(VerseGrammarCommand.StarterRevision, grammar.Revision); + Assert.Equal(VerseGrammarCommand.StarterDescription, grammar.Description); + } + // The shipped catalog carries the command's output. [Fact] public void The_shipped_catalog_holds_revision_one_and_the_seven_starter_verbs() @@ -318,6 +385,11 @@ public sealed class VerseGrammarTests : IDisposable Assert.Equal(2, factions.Length); Assert.All(factions, faction => Assert.Single(faction.Allegiance)); var adrasteia = cache.GetByName("Adrasteia"); - Assert.Equal(.75f, adrasteia.Allegiance[cache.RefOf(cache.GetByName("Brannoch"))]); + var brannoch = cache.GetByName("Brannoch"); + Assert.Equal(.75f, adrasteia.Allegiance[cache.RefOf(brannoch)]); + Assert.Equal(.25f, brannoch.Allegiance[cache.RefOf(adrasteia)]); + var speak = cache.GetByName("speak"); + Assert.Equal(VerseRenderPath.Conversation, speak.RenderPath); + Assert.Empty(speak.Roles); } }