Skip to content

fix(model): three entry constructs were dropped in silence - #5

Merged
mysma-9403 merged 1 commit into
mainfrom
fix/entries-that-were-read-as-if-unconditional
Oct 11, 2026
Merged

mysma-9403 merged 1 commit into
mainfrom
fix/entries-that-were-read-as-if-unconditional

Conversation

@mysma-9403

Copy link
Copy Markdown
Collaborator

This crate's whole claim is that a construct outside its subset is refused by name, never
quietly skipped
. Three of them were not.

construct what happened
<IncludeCondition> never read — the string did not appear anywhere in the crate. An entry present only when a condition holds was read as if it were always there.
<RepeatEntry> with a <DynamicValue> count fell through a chain that only looks for <FixedValue>, leaving repeat: None — which is also what no RepeatEntry at all leaves. The entry decoded exactly once.
a RepeatEntry or LocationInContainerInBits on an array/aggregate entry the early return that expands into one field per leaf happened before either was parsed, so both were discarded on the way past.

Dropping these is worse than it sounds. The entry keeps its place in the container, so its bits
are still claimed and every entry behind it is still read at an offset that assumes them — the
values that come out are wrong rather than missing, and nothing says so. This is precisely
the failure the Unsupported machinery exists to prevent.

All three are EntryKind::Unsupported now, reported at load, and pushed in place rather
than left out, so a caller sees something undecodable where the bits are instead of a container
quietly one entry short.

SUPPORTED.md had four rows that disagreed with the code — including ArrayParameterRefEntry
listed as unsupported on line 94 and as handled on line 176, which contradicted itself — and
now matches.

Why the differential suite never caught any of it: no bundled definition uses one.
rg RepeatEntry testdata/ returns nothing, and IncludeCondition appears in none of the nine.
This is the shape of gap that a test suite built from real mission files cannot close.

182 tests, all 9 differential cases still agree with the reference, fmt and clippy clean, and
the 1.85 floor was checked locally this time rather than discovered in CI.

This crate's whole claim is that a construct outside its subset is refused by
name rather than quietly skipped. Three of them were not.

* `<IncludeCondition>` was never read — the string did not appear anywhere in
  the crate. An entry present only when a condition holds was read as if it
  were always there.
* A `<RepeatEntry>` whose `<Count>` is a `<DynamicValue>` fell through a chain
  that only looks for `<FixedValue>`, leaving `repeat: None`, which is also
  what "no RepeatEntry at all" leaves. The entry decoded exactly once.
* An entry naming an array or an aggregate returns early to expand into one
  field per leaf, and the early return happened *before* the location and the
  repeat were parsed. Both were discarded on the way past.

Dropping these is worse than it sounds. The entry keeps its place in the
container, so its bits are still claimed and every entry behind it is still
read at an offset that assumes them: the values that come out are wrong rather
than missing, and nothing says so.

All three are `EntryKind::Unsupported` now, reported at load and pushed in
place rather than left out, so a caller sees something undecodable where the
bits are instead of a container quietly one entry short.

SUPPORTED.md had four rows that disagreed with the code — including
`ArrayParameterRefEntry` listed as unsupported on line 94 and as handled on
line 176 — and now matches it.

182 tests, all 9 differential cases still agree with the reference, fmt and
clippy clean, and this time the 1.85 floor was checked too.
@mysma-9403
mysma-9403 merged commit c4ddcc7 into main Oct 11, 2026
4 checks passed
@mysma-9403
mysma-9403 deleted the fix/entries-that-were-read-as-if-unconditional branch October 11, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant