From 96bbaa6320e68e1951e1d9fd228016dcb7b4f7ab Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Mon, 12 Oct 2026 20:00:18 +0200 Subject: [PATCH] validate shares Read's post-load composition; products, key, price rules pinned beyond the first product ShipAuthoringStore.ValidateLoaded is the one composition of hull Validate and ValidateProducts; Read and the draft branch of ship-authoring validate both call it (ValidateProducts is private). Tests: a draft with an unnamed hull is refused; blank product name and key collision with the offending product not first (both makers in turn); a negative hull price refused. Builds untested here; the verify job runs at the end. Co-Authored-By: Claude Opus 5.5 --- Assets/Scripts/ServerShared/ShipAuthoring.cs | 12 ++++++-- .../Aetheria.Shared.Tests/ShipProductTests.cs | 30 ++++++++++++++++--- tools/AetherDb/ShipAuthoringCommands.cs | 6 +--- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/Assets/Scripts/ServerShared/ShipAuthoring.cs b/Assets/Scripts/ServerShared/ShipAuthoring.cs index 42270fa6a..e21043182 100644 --- a/Assets/Scripts/ServerShared/ShipAuthoring.cs +++ b/Assets/Scripts/ServerShared/ShipAuthoring.cs @@ -205,13 +205,19 @@ public static class ShipAuthoringStore public static (HullData Hull, ShipAuthoring Visual, FactionProductData[] Products) Read(string path) { var (hull, visual, products) = Load(path); - Validate(hull, visual); - ValidateProducts(hull, visual, products); + ValidateLoaded(hull, visual, products); return (hull, visual, products); } + // Every judgement a loaded package's records face, in one place: Read, and validate on a draft with no model yet, call it. + public static void ValidateLoaded(HullData hull, ShipAuthoring visual, FactionProductData[] products) + { + Validate(hull, visual); + ValidateProducts(hull, visual, products); + } + // What a package may sell: every product named, no name twice, and a price above 0, because generation skips price 0. - public static void ValidateProducts(HullData hull, ShipAuthoring visual, FactionProductData[] products) + private static void ValidateProducts(HullData hull, ShipAuthoring visual, FactionProductData[] products) { if (products.Length == 0) return; if (products.Any(product => string.IsNullOrWhiteSpace(product.Name))) diff --git a/tests/Aetheria.Shared.Tests/ShipProductTests.cs b/tests/Aetheria.Shared.Tests/ShipProductTests.cs index ee33d49fc..f6f3bad10 100644 --- a/tests/Aetheria.Shared.Tests/ShipProductTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipProductTests.cs @@ -123,15 +123,23 @@ public sealed class ShipProductTests : IDisposable shipped.UpsertAsync(typeof(FactionProductData), held, ShipModCatalog.ProductKey("mod.keyed", _acme)).GetAwaiter().GetResult(); shipped.FlushAsync().Wait(); } - Package("mod.keyed", "Keyedhull", 100, Product("Fresh Name", _acme, "mod.keyed")); + using (var shipped = AetheriaStores.Open(Shipped, catalogWritable: true)) + { + var held = Product("Held Too", _rival, "mod.keyedr"); + shipped.UpsertAsync(typeof(FactionProductData), held, ShipModCatalog.ProductKey("mod.keyedr", _rival)).GetAwaiter().GetResult(); + shipped.FlushAsync().Wait(); + } + // The colliding product is one of two, and each maker takes its turn at being the colliding one, so the check + // cannot pass by looking at whichever product is listed first. + Package("mod.keyed", "Keyedhull", 100, Product("Fresh Name", _acme, "mod.keyed"), Product("Fresh Rebrand", _rival, "mod.keyed")); + Package("mod.keyedr", "Keyedrhull", 100, Product("Fresh Other", _acme, "mod.keyedr"), Product("Fresh Rival", _rival, "mod.keyedr")); Package("mod.fine", "Finehull", 100, Product("Fine", _acme, "mod.fine")); var composition = ShipModCatalog.Compose(Shipped, Derived, Mods); Assert.Equal(new[] { "mod.fine" }, composition.Included); - var excluded = Assert.Single(composition.Excluded); - Assert.Equal("mod.keyed", excluded.Package); - Assert.Contains("product key", excluded.Reason); + Assert.Equal(new[] { "mod.keyed", "mod.keyedr" }, composition.Excluded.Select(e => e.Package).OrderBy(p => p, StringComparer.Ordinal)); + Assert.All(composition.Excluded, e => Assert.Contains("product key", e.Reason)); } [Fact] @@ -194,6 +202,10 @@ public sealed class ShipProductTests : IDisposable Assert.Contains("price above 0", Assert.Throws(() => ShipAuthoringStore.Read(sold)).Message); Assert.Single(ShipAuthoringStore.Load(sold).Products); + // Below 0 is not sellable either. + var negative = Package("mod.negative", "Negative", -5, Product("Negative Skiff", _acme, "mod.negative")); + Assert.Contains("price above 0", Assert.Throws(() => ShipAuthoringStore.Read(negative)).Message); + // The smallest sellable price: the rule is "above 0", so 1 passes. Assert.Single(ShipAuthoringStore.Read(Package("mod.cheap", "Cheap", 1, Product("Cheap Skiff", _acme, "mod.cheap"))).Products); @@ -206,6 +218,12 @@ public sealed class ShipProductTests : IDisposable { var blank = Package("mod.blank", "Blank", 100, Product(" ", _acme, "mod.blank")); Assert.Contains("needs a name", Assert.Throws(() => ShipAuthoringStore.Read(blank)).Message); + // The unnamed product is one of two, once under each maker, so a check of the first product alone passes neither. + foreach (var (id, unnamed, named) in new[] { ("mod.blank-acme", _acme, _rival), ("mod.blank-rival", _rival, _acme) }) + { + var second = Package(id, id, 100, Product("Named", named, id), Product(" ", unnamed, id)); + Assert.Contains("needs a name", Assert.Throws(() => ShipAuthoringStore.Read(second)).Message); + } var twice = Package("mod.twice", "Twice", 100, Product("Same", _acme, "mod.twice"), Product("Same", _rival, "mod.twice")); Assert.Contains("unique", Assert.Throws(() => ShipAuthoringStore.Read(twice)).Message); } @@ -302,6 +320,10 @@ public sealed class ShipProductTests : IDisposable Assert.Contains("needs a name", Refusal(Draft(Package("mod.blank", "Blank", 100, Product(" ", _acme, "mod.blank"))))); Assert.Contains("unique", Refusal(Draft(Package("mod.twice", "Twice", 100, Product("Same", _acme, "mod.twice"), Product("Same", _rival, "mod.twice"))))); + // The hull's own rules judge a draft too: validate runs the one composition Read runs. + var unnamedHull = Draft(Path.Combine(ShipFixture.WritePackage(Mods, "mod.unnamed", hullName: ""), "ship.cc")); + Assert.Contains("hull name is required", Refusal(unnamedHull)); + var sound = Draft(Package("mod.sound", "Sound", 100, Product("Sound Skiff", _acme, "mod.sound"), Product("Sound Rebrand", _rival, "mod.sound"))); var (validated, output, _) = Run("validate", sound); Assert.Equal(0, validated); diff --git a/tools/AetherDb/ShipAuthoringCommands.cs b/tools/AetherDb/ShipAuthoringCommands.cs index ce016dd16..9fd7e690a 100644 --- a/tools/AetherDb/ShipAuthoringCommands.cs +++ b/tools/AetherDb/ShipAuthoringCommands.cs @@ -49,11 +49,7 @@ public static class ShipAuthoringCommands : Path.Combine(Path.GetDirectoryName(Path.GetFullPath(args[1])), ship.ModelAsset); var judged = model != null && File.Exists(model) ? "records and model" : "records only, no model yet"; if (judged == "records and model") ShipModCatalog.ReadPackage(args[1]); - else - { - ShipAuthoringStore.Validate(hull, ship); - ShipAuthoringStore.ValidateProducts(hull, ship, products); - } + else ShipAuthoringStore.ValidateLoaded(hull, ship, products); Console.WriteLine($"{ship.Id}: {hull.Name}, {hull.Hardpoints.Count} hardpoints, {ship.Anchors.Count} anchors, {ship.SchematicLines.Count} lines ({judged})"); return 0; }