From 0b5cad890370958f0893b5eeb490d856295d4769 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Sat, 10 Oct 2026 09:49:39 +0200 Subject: [PATCH] ship_cc: layout_revision and valid_ship_id as pure functions, fixed-text errors; pins and tests for the authoring-tool Low findings layout_revision(shape, hardpoints) is the revision rule replace_layout and Package share. SHIP_ID_CHARS and RESERVED_SHIP_IDS mirror ShipAuthoringStore.RequireShipId; ShipSchemaPinTests sweeps every character and every candidate stem against the C# rule. ship_cc errors no longer carry the path, mount id, anchor id or object name. C# tests: create judges the ID not the display name, FindRoot answers the working directory itself, short trailing-dot IDs. Builds: yes (C# test edits and Python only). Co-Authored-By: Claude Opus 5.5 --- .../AetherDbRootTests.cs | 9 ++++ .../ShipModCatalogTests.cs | 12 ++++- .../ShipSchemaPinTests.cs | 33 +++++++++++++ .../ShipValidationTests.cs | 3 +- tools/blender/aetheria_ships/ship_cc.py | 46 +++++++++++++------ tools/blender/tests/test_ship_cc.py | 31 ++++++++++++- 6 files changed, 115 insertions(+), 19 deletions(-) diff --git a/tests/Aetheria.Shared.Tests/AetherDbRootTests.cs b/tests/Aetheria.Shared.Tests/AetherDbRootTests.cs index f417d4e10..6aa614182 100644 --- a/tests/Aetheria.Shared.Tests/AetherDbRootTests.cs +++ b/tests/Aetheria.Shared.Tests/AetherDbRootTests.cs @@ -29,6 +29,15 @@ public sealed class AetherDbRootTests : IDisposable Assert.Equal(game, AetherDb.FindRoot()); } + [Fact] + public void TheRootIsTheWorkingDirectoryItselfWhenItsGameDataHoldsAetheriaCc() + { + Directory.CreateDirectory(Path.Combine(_game.Path, "GameData")); + File.WriteAllBytes(Path.Combine(_game.Path, "GameData", "Aetheria.cc"), Array.Empty()); + Environment.CurrentDirectory = _game.Path; + Assert.Equal(Environment.CurrentDirectory, AetherDb.FindRoot()); + } + [Fact] public void AFolderHoldingOnlyTheRepositoryMarkerIsNotARoot() { diff --git a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs index 83f9b9ad1..7e025792c 100644 --- a/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipModCatalogTests.cs @@ -424,12 +424,22 @@ public sealed class ShipModCatalogTests : IDisposable public void CreateRefusesABadIdBeforeTouchingTheDisk(string id) { var target = Path.Combine(_directory.Path, "GameData", "Mods", id, "ship.cc"); - var (code, output, error) = Run("create", target, id, "Probe", "--like", "Djinni"); + // The display name is itself a valid ship ID, so a rule applied to it instead of the ID would not refuse. + var (code, output, error) = Run("create", target, id, "probe", "--like", "Djinni"); Assert.Equal(1, code); Assert.Contains("Ship ID", error); Assert.False(Directory.Exists(Path.Combine(_directory.Path, "GameData"))); } + [Fact] + public void CreateJudgesTheIdAndTakesAnyDisplayName() + { + var target = Path.Combine(_directory.Path, "GameData", "Mods", "probe.a", "ship.cc"); + var (code, output, error) = Run("create", target, "probe.a", "Probe A"); + Assert.Equal(0, code); + Assert.True(File.Exists(target), error); + } + private static (int Code, string Output, string Error) Run(params string[] args) { var (output, error) = (new StringWriter(), new StringWriter()); diff --git a/tests/Aetheria.Shared.Tests/ShipSchemaPinTests.cs b/tests/Aetheria.Shared.Tests/ShipSchemaPinTests.cs index e674ed23a..9e690828a 100644 --- a/tests/Aetheria.Shared.Tests/ShipSchemaPinTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipSchemaPinTests.cs @@ -67,6 +67,39 @@ public sealed class ShipSchemaPinTests Assert.Equal(slot, KeyOf(typeof(ShipAnchor), members[slot])); } + // New Ship refuses a bad ship ID before it touches anything, with ship_cc.valid_ship_id; these two constants are that + // rule's data, so the C# rule that create and Validate share must agree with them on every character and every stem. + [Fact] + public void PythonShipIdConstantsAreTheRuleRequireShipIdEnforces() + { + var allowed = Text("SHIP_ID_CHARS"); + var reserved = Strings("RESERVED_SHIP_IDS"); + for (var code = 0; code < 256; code++) + { + var c = (char)code; + Assert.Equal(allowed.Contains(c), Accepts("a" + c)); + Assert.Equal(allowed.Contains(c) && c != '.' && c != '_' && c != '-', Accepts(c + "a")); + } + var stems = new List(); + for (var a = 'a'; a <= 'z'; a++) + for (var b = 'a'; b <= 'z'; b++) + for (var c = 'a'; c <= 'z'; c++) + stems.Add($"{a}{b}{c}"); + for (var digit = 0; digit <= 9; digit++) + stems.AddRange(new[] { $"com{digit}", $"lpt{digit}" }); + foreach (var stem in stems) + { + Assert.Equal(!reserved.Contains(stem), Accepts(stem)); + Assert.Equal(!reserved.Contains(stem), Accepts(stem + ".x")); + } + } + + private static bool Accepts(string id) + { + try { ShipAuthoringStore.RequireShipId(id); return true; } + catch (InvalidOperationException) { return false; } + } + [Fact] public void PythonEnumNamesAreTheEnumsInValueOrder() { diff --git a/tests/Aetheria.Shared.Tests/ShipValidationTests.cs b/tests/Aetheria.Shared.Tests/ShipValidationTests.cs index 7081c58f9..228c60494 100644 --- a/tests/Aetheria.Shared.Tests/ShipValidationTests.cs +++ b/tests/Aetheria.Shared.Tests/ShipValidationTests.cs @@ -177,7 +177,8 @@ public sealed class ShipValidationTests Add("id-blank", s => s.Visual.Id = " ", "Ship ID is required"); foreach (var device in new[] { "con", "prn", "aux", "nul", "nul.x", "com1", "com9", "lpt1", "lpt9", "con.txt" }) Add("id-device-" + device, s => s.Visual.Id = device, "Windows device name"); - Add("id-trailing-dot", s => s.Visual.Id = "abc.", "must not end with a dot"); + foreach (var id in new[] { "abc.", "a.", "ab.", "a.b." }) + Add("id-trailing-dot-" + id, s => s.Visual.Id = id, "must not end with a dot"); Add("hull-null", s => s.Hull = null, "hull data is required"); Add("hull-name-null", s => s.Hull.Name = null, "hull name is required"); Add("hull-name-blank", s => s.Hull.Name = " ", "hull name is required"); diff --git a/tools/blender/aetheria_ships/ship_cc.py b/tools/blender/aetheria_ships/ship_cc.py index 6018d6a96..df76cdb6f 100644 --- a/tools/blender/aetheria_ships/ship_cc.py +++ b/tools/blender/aetheria_ships/ship_cc.py @@ -37,6 +37,21 @@ HARDPOINT_TYPE_NAMES = ("Hull", "Tool", "Thermal", "Thruster", "WarpDrive", "Rea ROTATION_NAMES = ("None", "CounterClockwise", "Reversed", "Clockwise") +# ShipAuthoringStore.RequireShipId's rule, mirrored so New Ship refuses before it touches anything; ShipSchemaPinTests +# pins both constants and valid_ship_id to the C# rule. +SHIP_ID_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789._-" +RESERVED_SHIP_IDS = ("con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8", "com9", + "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9") + + +def valid_ship_id(ship_id: str) -> bool: + """True when ShipAuthoringStore.RequireShipId would accept the ID: lower-case ASCII letters, digits, dots, + underscores and hyphens, starting with a letter or digit, not ending in a dot, and whose stem before the first dot + is not a Windows device name.""" + return (bool(ship_id) and ship_id[0] not in "._-" and all(c in SHIP_ID_CHARS for c in ship_id) + and not ship_id.endswith(".") and ship_id.split(".")[0] not in RESERVED_SHIP_IDS) + + def decode_hardpoint(row: list[Any]) -> dict[str, Any]: return dict(zip(HARDPOINT_MEMBERS, row)) @@ -80,19 +95,19 @@ def read(path: str, cultlib_packages: str) -> ShipFile: body = msgpack.unpackb(envelope.payload, raw=False) if isinstance(body, list) and len(body) > RETIRED_HULL_SLOT and body[RETIRED_HULL_SLOT] is not None: raise ValueError( - f"{path}: {envelope.key}: legacy embedded hull at retired key {RETIRED_HULL_SLOT} of the {SCHEMA} record; " + f"The ship file has a legacy embedded hull at retired key {RETIRED_HULL_SLOT} of the {SCHEMA} record; " f"migrate it into its own {HULL_SCHEMA} record that names the visual, and drop key {RETIRED_HULL_SLOT}") if len(envelopes) != 2 or len(ships) != 1 or len(hulls) != 1: - raise ValueError(f"{path} must hold exactly one {SCHEMA} record and one {HULL_SCHEMA} record") + raise ValueError(f"The ship file must hold exactly one {SCHEMA} record and one {HULL_SCHEMA} record") bodies = [] for envelope in (ships[0], hulls[0]): body = msgpack.unpackb(envelope.payload, raw=False) if not isinstance(body, list): - raise ValueError(f"{path} has an incompatible {envelope.type} payload") + raise ValueError(f"The ship file has an incompatible {envelope.type} payload") bodies.append(Record(envelope, body)) ship, hull = bodies if len(ship.body) < SCHEMATIC_LINES_SLOT + 1: - raise ValueError(f"{path} has an incompatible {SCHEMA} payload") + raise ValueError(f"The ship file has an incompatible {SCHEMA} payload") return ShipFile(store, ship, hull, msgpack) @@ -129,7 +144,7 @@ def replace_visual(path: str, cultlib_packages: str, expected_id: str, model_ass if not isinstance(anchor_id, str) or not anchor_id.strip(): raise ValueError("Every anchor needs an ID") if anchor_id in ids: - raise ValueError(f"Anchor ID {anchor_id} is used twice") + raise ValueError("An anchor ID is used twice") ids.add(anchor_id) previous = file.ship.body[ANCHORS_SLOT] if len(file.ship.body) > ANCHORS_SLOT else None later = {row[0]: row[known:] for row in previous or [] @@ -149,13 +164,14 @@ def _hull_body(file: ShipFile) -> list: def read_layout(path: str, cultlib_packages: str): file = read(path, cultlib_packages) hull = _hull_body(file) - return hull[HULL_SHAPE_SLOT][0], hull[HULL_HARDPOINTS_SLOT], _layout_revision(hull, file.msgpack) + return hull[HULL_SHAPE_SLOT][0], hull[HULL_HARDPOINTS_SLOT], layout_revision(hull[HULL_SHAPE_SLOT][0], hull[HULL_HARDPOINTS_SLOT]) -def _layout_revision(hull, msgpack): - return hashlib.sha256(msgpack.packb( - [hull[HULL_SHAPE_SLOT], hull[HULL_HARDPOINTS_SLOT]], use_bin_type=True - )).hexdigest() +def layout_revision(shape: list[Any], hardpoints: list[list[Any]]) -> str: + """The revision of a layout: a hash of its shape ([width, height, cells]) and hardpoint rows as the file stores + them. replace_layout returns it, Load records it, and Package compares the layout buffer's against the file's.""" + import msgpack # type: ignore + return hashlib.sha256(msgpack.packb([[shape], hardpoints], use_bin_type=True)).hexdigest() def replace_layout(path: str, cultlib_packages: str, expected_id: str, expected_revision: str, @@ -164,7 +180,7 @@ def replace_layout(path: str, cultlib_packages: str, expected_id: str, expected_ if file.ship.body[0] != expected_id: raise ValueError("The bound ship ID changed; reload its layout") hull = _hull_body(file) - if _layout_revision(hull, file.msgpack) != expected_revision: + if layout_revision(hull[HULL_SHAPE_SLOT][0], hull[HULL_HARDPOINTS_SLOT]) != expected_revision: raise ValueError("The .cc layout changed since Load; reload before saving") width, height, cells = shape if not (1 <= width <= 32 and 1 <= height <= 32 and len(cells) == width * height): @@ -184,20 +200,20 @@ def replace_layout(path: str, cultlib_packages: str, expected_id: str, expected_ raise ValueError(f"Each hardpoint needs {known} typed fields and a stable mount ID") hp_width, hp_height, hp_cells = fields["Shape"][0] if not all(type(cell) is bool for cell in hp_cells): - raise ValueError(f"Hardpoint {mount} footprint cells must be booleans") + raise ValueError("Hardpoint footprint cells must be booleans") if not (1 <= hp_width <= 32 and 1 <= hp_height <= 32 and len(hp_cells) == hp_width * hp_height and any(hp_cells)): - raise ValueError(f"Hardpoint {mount} has an invalid footprint") + raise ValueError("A hardpoint has an invalid footprint") updated_hardpoints.append(hardpoint + previous.get(mount, [])) hull[HULL_SHAPE_SLOT] = [shape] hull[HULL_HARDPOINTS_SLOT] = updated_hardpoints file.store.push(_stamped(file.hull, file.msgpack)) - return _layout_revision(hull, file.msgpack) + return layout_revision(hull[HULL_SHAPE_SLOT][0], hull[HULL_HARDPOINTS_SLOT]) def capture_grease_pencil(obj: Any, depsgraph: Any, frame_number: int, *, evaluated: bool = True) -> list[list[Any]]: if obj.type != "GREASEPENCIL": - raise ValueError(f"{obj.name} is not a Grease Pencil object") + raise ValueError("The selected object is not a Grease Pencil object") source = obj.evaluated_get(depsgraph) if evaluated else obj lines: list[list[Any]] = [] for layer in source.data.layers: diff --git a/tools/blender/tests/test_ship_cc.py b/tools/blender/tests/test_ship_cc.py index d0f80150a..4a9c045d8 100644 --- a/tools/blender/tests/test_ship_cc.py +++ b/tools/blender/tests/test_ship_cc.py @@ -311,6 +311,16 @@ class LayoutTests(ShipFileCase): self.write(ship, hull) self.assertNotEqual(revision, self.layout()[2]) + def test_layout_revision_is_the_revision_replace_layout_returns_for_the_same_shape_and_rows(self): + _, _, revision = self.layout() + cells = [True, True, True, False] + rows = [hardpoint_row("thruster.starboard", tail=())] # a mount the file carried no later slots for + saved = self.edit(revision, cells=cells, hardpoints=rows) + self.assertEqual(saved, ship_cc.layout_revision(shape(2, 2, cells), rows)) + self.assertEqual(saved, self.layout()[2]) + self.assertNotEqual(saved, ship_cc.layout_revision(shape(2, 2, [True, True, False, False]), rows)) + self.assertNotEqual(saved, ship_cc.layout_revision(shape(2, 2, cells), [])) + def test_the_revision_follows_the_hardpoints_too(self): _, _, revision = self.layout() moved = hardpoint_row(tail=()) @@ -419,7 +429,7 @@ class LayoutTests(ShipFileCase): row = ship_cc.encode_hardpoint(**{**ship_cc.decode_hardpoint(hardpoint_row(tail=())), "Shape": [shape(2, 1, cells)]}) with self.subTest(name): - with self.assertRaisesRegex(ValueError, "thruster.port footprint cells must be booleans"): + with self.assertRaisesRegex(ValueError, "Hardpoint footprint cells must be booleans"): self.edit(revision, hardpoints=[row]) self.assertEqual(before, self.bytes()) @@ -456,7 +466,7 @@ class LayoutTests(ShipFileCase): ("short", shape(2, 1, [True])), ("empty", shape(1, 1, [False]))): row = ship_cc.encode_hardpoint(**{**ship_cc.decode_hardpoint(hardpoint_row(tail=())), "Shape": [footprint]}) with self.subTest(name): - with self.assertRaisesRegex(ValueError, "thruster.port has an invalid footprint"): + with self.assertRaisesRegex(ValueError, "hardpoint has an invalid footprint"): self.edit(revision, hardpoints=[row]) self.assertEqual(before, self.bytes()) @@ -509,6 +519,23 @@ class CSharpWrittenFixtureTests(unittest.TestCase): shutil.copy(Path(os.environ["SHIP_FIXTURE_DIR"]) / "mod.skiff" / "skiff.glb", target / "skiff.glb") +class ShipIdTests(unittest.TestCase): + def test_admits_the_ids_the_c_sharp_rule_admits(self): + for ship_id in ("probe.a", "a", "0x", "a_b-c", "com10", "com0", "lpt0", "console", "aux2", "a.nul", "x.com1"): + with self.subTest(ship_id): + self.assertTrue(ship_cc.valid_ship_id(ship_id)) + + def test_refuses_device_names_alone_or_before_a_dot_and_every_other_malformed_id(self): + for ship_id in ("con", "nul.x", "com1", "lpt9", "prn", "aux.txt", "com5.a", "abc.", "a..", "Upper", ".x", "_x", + "-x", "", "a b", "a/b", "a:b", "abé", "a\b"): + with self.subTest(ship_id): + self.assertFalse(ship_cc.valid_ship_id(ship_id)) + + def test_the_constants_name_the_allowed_characters_and_the_reserved_stems(self): + self.assertEqual(set("abcdefghijklmnopqrstuvwxyz0123456789._-"), set(ship_cc.SHIP_ID_CHARS)) + self.assertEqual(22, len(ship_cc.RESERVED_SHIP_IDS)) + + class HardpointRowTests(unittest.TestCase): def test_a_row_is_one_value_per_member_in_slot_order(self): fields = dict(Type=1, Position=[2, 3], Shape="shape", Transform="mount", Rotation=2, Armor=4.0, FiringArc=90.0)