From fe38f28865d2f0d8ae5662dd3fbbcd023d0348f3 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Wed, 30 Sep 2026 02:14:10 +0200 Subject: [PATCH] Close the gaps a mutation pass found in the new ship tests (S-Soul F1-F3) A pass of 137 hand-written mutations over ShipAuthoring.cs, ShipModCatalog.cs and ship_cc.py, run against the tests from the last three commits, left six surviving or unrunnable. Each is resolved: - Validator: the hull Width/Height < 1 guards and the opacity IsInfinity guard could not change any outcome (an empty grid has no occupied cell; +Infinity exceeds 1 and -Infinity is below 0). Deleted. - Two accepted shapes had no test: a hardpoint footprint with empty cells beside occupied ones, and an empty ParentId meaning no parent. Added. - The GLB header length was pinned only from above. Added the shorter case. - Compose's working-directory cleanup was invisible because the assertion listed files, not directory entries. It now lists both. - ReadPackage runs the catalog's item validation on the hull (the derived catalog must reopen); that call now has a test. - Python: a single non-ship record, a non-array payload long enough to fill every slot, and a hardpoint-only change to the layout revision each had a surviving mutation. Tests added. Co-Authored-By: Claude Opus 5.5 --- Assets/Scripts/ServerShared/ShipAuthoring.cs | 5 ++-- .../ShipModCatalogTests.cs | 17 +++++++++++--- .../ShipValidationTests.cs | 17 ++++++++++++++ tools/blender/tests/test_ship_cc.py | 23 ++++++++++++++++--- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/Assets/Scripts/ServerShared/ShipAuthoring.cs b/Assets/Scripts/ServerShared/ShipAuthoring.cs index c19ab153d..992e6405f 100644 --- a/Assets/Scripts/ServerShared/ShipAuthoring.cs +++ b/Assets/Scripts/ServerShared/ShipAuthoring.cs @@ -83,8 +83,7 @@ public static class ShipAuthoringStore throw new InvalidOperationException($"{ship.Id}: ship ID must use lower-case ASCII letters, digits, dots, underscores, or hyphens."); var hull = ship.Hull ?? throw new InvalidOperationException($"{ship.Id}: hull data is required."); if (string.IsNullOrWhiteSpace(hull.Name)) throw new InvalidOperationException($"{ship.Id}: hull name is required."); - if (hull.Shape?.Cells == null || hull.Shape.Width < 1 || hull.Shape.Height < 1 || - !hull.Shape.Cells.Cast().Any(occupied => occupied)) + if (hull.Shape?.Cells == null || !hull.Shape.Cells.Cast().Any(occupied => occupied)) throw new InvalidOperationException($"{ship.Id}: schematic must contain at least one cell."); if (!string.IsNullOrEmpty(hull.Prefab)) throw new InvalidOperationException($"{ship.Id}: a mod ship cannot name a Unity prefab."); @@ -146,7 +145,7 @@ public static class ShipAuthoringStore line.Opacities != null && line.Opacities.Length != count) throw new InvalidOperationException($"{ship.Id}: schematic line point attributes have the wrong length."); if (line.Radii != null && line.Radii.Any(value => float.IsNaN(value) || float.IsInfinity(value) || value < 0) || - line.Opacities != null && line.Opacities.Any(value => float.IsNaN(value) || float.IsInfinity(value) || value < 0 || value > 1)) + line.Opacities != null && line.Opacities.Any(value => float.IsNaN(value) || value < 0 || value > 1)) throw new InvalidOperationException($"{ship.Id}: schematic line radius and opacity must be finite and in range."); if (line.Color != null && (line.Color.Length != 4 || line.Color.Any(value => float.IsNaN(value) || float.IsInfinity(value)))) diff --git a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs index 3c651f38e..029ea81c8 100644 --- a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs @@ -205,6 +205,14 @@ public sealed class ShipModCatalogTests : IDisposable ShipModCatalog.ReadPackage(Path.Combine(Mods, "mod.skiff", "ship.cc"))).Message); } + [Fact] + public void ReadPackageAppliesTheCatalogsItemValidationToTheHull() + { + WritePackage("mod.skiff", tweak: ship => ship.Hull.MinimumTemperature = ship.Hull.MaximumTemperature); + Assert.Contains("zero-span range", Assert.Throws(() => + ShipModCatalog.ReadPackage(Path.Combine(Mods, "mod.skiff", "ship.cc"))).Message); + } + [Fact] public void ReadPackageRefusesAnAnchorWhoseNodeIsNotInTheModel() { @@ -240,7 +248,8 @@ public sealed class ShipModCatalogTests : IDisposable Refuse("short.glb", With(good.Take(19).ToArray(), 8, 19), "invalid GLB 2 header"); Refuse("magic.glb", With(good, 0, 0x46546C68), "invalid GLB 2 header"); Refuse("version.glb", With(good, 4, 1), "invalid GLB 2 header"); - Refuse("length.glb", With(good, 8, (uint)good.Length + 1), "invalid GLB 2 header"); + Refuse("longer.glb", With(good, 8, (uint)good.Length + 1), "invalid GLB 2 header"); + Refuse("shorter.glb", With(good, 8, (uint)good.Length - 4), "invalid GLB 2 header"); Refuse("chunktype.glb", With(good, 16, 0x004E4942), "missing GLB JSON chunk"); Refuse("chunklength.glb", With(good, 12, (uint)good.Length), "missing GLB JSON chunk"); } @@ -261,7 +270,7 @@ public sealed class ShipModCatalogTests : IDisposable private void AssertOnlyTheDerivedFile() { - var names = Directory.GetFiles(Path.GetDirectoryName(Derived)).Select(file => Path.GetFileName(file)).ToArray(); + var names = Directory.GetFileSystemEntries(Path.GetDirectoryName(Derived)).Select(entry => Path.GetFileName(entry)).ToArray(); Assert.True(names.Length == 1 && names[0] == "Aetheria.modded.cc", string.Join(" | ", names)); } @@ -297,9 +306,11 @@ public sealed class ShipModCatalogTests : IDisposable return path; } - private void WritePackage(string id, string hullName = "Skiff", string[] nodes = null, string modelAsset = "skiff.glb") + private void WritePackage(string id, string hullName = "Skiff", string[] nodes = null, string modelAsset = "skiff.glb", + Action tweak = null) { var ship = ShipAuthoringTests.Fixture(); + tweak?.Invoke(ship); ship.Id = id; ship.Hull.Name = hullName; ship.ModelAsset = modelAsset; diff --git a/tests/Aetheria.Shared.Tests/ShipValidationTests.cs b/tests/Aetheria.Shared.Tests/ShipValidationTests.cs index d8d94b8a6..17d54ad4b 100644 --- a/tests/Aetheria.Shared.Tests/ShipValidationTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipValidationTests.cs @@ -96,6 +96,23 @@ public sealed class ShipValidationTests ShipAuthoringStore.Validate(ship); } + [Fact] + public void EmptyCellsInAHardpointFootprintNeedNotLieOnTheHull() + { + var ship = ShipAuthoringTests.Fixture(); + ship.Hull.Hardpoints[0].Shape = new Shape(2, 1); + ship.Hull.Hardpoints[0].Shape.Cells[0, 0] = true; + ShipAuthoringStore.Validate(ship); + } + + [Fact] + public void AnEmptyParentIdMeansNoParent() + { + var ship = ShipAuthoringTests.Fixture(); + ship.Anchors[0].ParentId = ""; + ShipAuthoringStore.Validate(ship); + } + [Fact] public void AnAnchorMayNameItsHardpointAsParent() { diff --git a/tools/blender/tests/test_ship_cc.py b/tools/blender/tests/test_ship_cc.py index ab9eb6299..2e6715822 100644 --- a/tools/blender/tests/test_ship_cc.py +++ b/tools/blender/tests/test_ship_cc.py @@ -100,6 +100,13 @@ class ReadTests(ShipFileCase): with self.assertRaisesRegex(ValueError, "must hold exactly one"): ship_cc.read(self.path, PACKAGES) + def test_refuses_a_file_whose_only_record_is_not_a_ship(self): + other = str(Path(self.path).with_name("other.cc")) + cultcache_py.SingleFileMessagePackBackingStore(other).push(cultcache_py.CultCacheEnvelope.create( + key="other", type="aetheria.something", payload=msgpack.packb([1], use_bin_type=True))) + with self.assertRaisesRegex(ValueError, "must hold exactly one"): + ship_cc.read(other, PACKAGES) + def test_refuses_a_file_with_no_record(self): empty = str(Path(self.path).with_name("empty.cc")) Path(empty).write_bytes(b"") @@ -112,9 +119,11 @@ class ReadTests(ShipFileCase): ship_cc.read(self.path, PACKAGES) def test_refuses_a_payload_that_is_not_an_array(self): - self.write({"Id": "mod.skiff"}) - with self.assertRaisesRegex(ValueError, "incompatible"): - ship_cc.read(self.path, PACKAGES) + for payload in ({"Id": "mod.skiff"}, {str(index): index for index in range(9)}, "a string long enough to fill every slot"): + with self.subTest(payload=payload): + self.write(payload) + with self.assertRaisesRegex(ValueError, "incompatible"): + ship_cc.read(self.path, PACKAGES) class ReplaceLinesTests(ShipFileCase): @@ -168,6 +177,14 @@ class LayoutTests(ShipFileCase): self.write(body) self.assertNotEqual(revision, self.layout()[2]) + def test_the_revision_follows_the_hardpoints_too(self): + _, _, revision = self.layout() + moved = hardpoint_row(tail=()) + moved[1] = [0, 0] + same_cells = [True, False, True, False] + self.assertEqual(revision, self.edit(revision, cells=same_cells, hardpoints=[hardpoint_row(tail=())])) + self.assertNotEqual(revision, self.edit(revision, cells=same_cells, hardpoints=[moved])) + def test_saves_the_layout_and_returns_the_new_revision(self): _, _, revision = self.layout() moved = hardpoint_row(tail=())