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 <noreply@anthropic.com>
This commit is contained in:
Meta Vens
parent
fb030ca9c3
commit
0b5cad8903
6 changed files with 115 additions and 19 deletions
|
|
@ -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<byte>());
|
||||
Environment.CurrentDirectory = _game.Path;
|
||||
Assert.Equal(Environment.CurrentDirectory, AetherDb.FindRoot());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AFolderHoldingOnlyTheRepositoryMarkerIsNotARoot()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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<string>();
|
||||
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()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue