feat: add ISO-backed framedata runtime mapping #23
No reviewers
Labels
No labels
area/contracts
area/documentation
area/observability
area/testing
good-first-slice
priority
p0
priority
p1
priority
p2
priority
p3
risk/legal
risk/security
status
blocked
type
epic
type
feature
type
spike
type
task
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
LiveMindIO/libmelee!23
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/iso-framedata-runtime-mapping"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Character/Actionstates through auditedmain.dolMotionState.anim_idandmove_idtables with validated callback metadataTransN/TransN2animation-root samples, adjacent deltas, scale-selection flags, and facing projection without presenting them as runtime locomotionFrameData.roll_end_position()works from ISO root curves while tech and ledge rolls fail explicitlyFrameData.iso_pathand high-levelget_framedata()queries select the ISO throughMELEE_ISO_PATH, with source-keyed caches and validated disc provenanceNoneget_raw_framedata_csv()explicitly historical and fail closed for article/projectile-dependent ISO timing instead of returning partial grouped-special resultsFrameDatawhile explicitly rejecting unsupported historical runtime geometry and ambiguous article hitboxesValidation
python test.py(389 tests)python -m compileall -q melee test.py1e-64.58e-05Current Limits
get_raw_framedata_csv()and remaining direct.framedatadictionary consumers still depend on the historical CSV during incremental migrationFound four actionable correctness issues in the ISO-backed runtime mapping and subaction interpretation.
@ -0,0 +350,4 @@values.update(_fields(words, specs))if opcode == 11:values["size"] = values["size_raw"] / 256.0values["bone_local_x"] = values["bone_local_x_raw"] / 256.0[P2] These effective axes are reversed relative to the runtime collision vector:
ftAction_8007121Cassigns encodedz_offsettoHitCapsule.b_offset.xand encodedx_offsettob_offset.z, while this maps source X to public X and source Z to public Z. For example, retail Fox jab is reported as(x=0, z=4.65625)although the runtime vector is(x=4.65625, z=0). Please swap the effective X/Z assignments (retaining separately named raw fields if needed) and correct the synthetic assertion.@ -0,0 +505,4 @@target = int(command.parameter("target"))if target % 4 or target < 0 or target >= len(data):raise SubactionParseError(f"{context}: opcode {opcode} targets invalid data offset 0x{target:X}")if opcode == 7 and target <= pc:[P1] Treating every backward
gotoas a loop skips valid tail jumps into earlier shared routines. On the retail ISO, FoxFTILT_HIGHconsists of a backward jump to the terminatingFTILT_MIDscript, but this returns only opcode 7 and no hitboxes, making ISO-backedis_attack()false instead of exposing the frame-5 hitboxes. Captain Falcon'sFSMASH_MIDsimilarly skips the shared cleanup and leaves its hitboxes active through frame 64 instead of frame 21. Please follow backward targets and classify a loop only after detecting actual repeated control flow, with a terminating backward-jump regression test.@ -0,0 +322,4 @@dat_index = self.dat_action_index(character, action)if dat_index is None:return Nonereturn self.action(FIGHTER_KINDS[character.value].code, dat_index)[P2] This direct
PlNn.datlookup skips Nana's runtime fallback.ftData_80085FD4selects Popo's corresponding action record for non-demo Nana whenever Nana'sx14animation pointer is null; on the retail ISO, Nana jab therefore returns a 16-frame script-only timeline here instead of the inherited 30-frame endpoint/29 snapshots, with the same problem affecting grab and rolls. Please apply the Nana-to-Popo record fallback used by the engine and cover an inherited common action.[P3] A mapped DAT index can still denote an absent animation, but this always returns its empty record despite the documented
Nonecontract. Retail FoxNEUTRAL_ATTACK_3maps to index 48, whose animation size, runtime animation pointer, and script are all zero, yetaction_for_state()returns an emptyActionRecord. After applying fighter-specific fallback rules, please returnNonefor mapped-but-empty actions and test this common-state case.Review comment #651: Fixed in
2202f97. Backward gotos now execute until they actually re-enter previously seen control flow, so terminating shared routines are preserved. Added a higher-address-to-lower-address terminating tail-jump regression. Focused tests:python -m unittest test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry(2 passed). Retail verification now reports Fox FTILT_HIGH active on frames 5-8 and Captain Falcon FSMASH_MID active on frames 18-21. Full branch:python test.py(335 passed).Review comment #652: Fixed in
2202f97. Effective hitbox X now uses the encodedz_offset, and effective Z uses encodedx_offset, matchingftAction_8007121C; the synthetic geometry assertions were corrected. Focused tests:python -m unittest test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry(2 passed). Retail Fox jab now reports(x=4.65625, z=0). Full branch:python test.py(335 passed);uvx ruff check melee/_subaction.py melee/disc_framedata.pypassed.Review comment #653: Fixed in
3f97918, with the article-action fixture correction in7628676.action_for_state()now resolves a Nana entry with no local animation payload through the corresponding Popo record, while low-levelaction("Nn", index)remains unchanged. Added synthetic Nana/Popo fallback coverage. Focused tests:python -m unittest test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_nana_runtime_actions_inherit_missing_popo_animations(2 passed). Retail verification now gives Nana jab/grab/forward-roll timelines of 29/29/31 snapshots. Full branch:python test.py(335 passed).Review comment #654: Fixed in
3f97918, with the animated-article fixture correction in7628676. After fighter-specific fallback,action_for_state()now returnsNonewhen the resolved DAT entry has no animation payload; animated article-producing states retain their explicit unsupported-hitbox error. Added mapped-but-empty Fox and article-state coverage. Focused tests:python -m unittest test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_nana_runtime_actions_inherit_missing_popo_animations(3 passed). Retail Fox NEUTRAL_ATTACK_3 now returnsNone; fullpython test.pypassed 335 tests.Found four actionable correctness issues in the ISO-backed runtime mapping and compatibility facade.
@ -0,0 +470,4 @@pc += 4continueif opcode == 2:next_time = max(time, float(command.parameter("frame")))[P2]
max(time, target)discards the runtime timer's negative remainder when an asynchronous timer targets an earlier frame, so later synchronous waits are added to elapsed time instead of the remaining timer. Retail YoshiDAIRconsequently reports IASA on frame 56 instead of 53, and SamusBAT_SWING_3shifts its hitboxes from frames 13-15 to 16-18. Please track the command timer separately from elapsed animation time and cover a past absolute timer followed by relative timers.@ -0,0 +325,4 @@record = self.action(FIGHTER_KINDS[character.value].code, dat_index)if character is Character.NANA and not record.animation_size:record = self.action(FIGHTER_KINDS[Character.POPO.value].code, dat_index)return record if record.animation_size else None[P2] Filtering solely on
animation_sizedrops executable script-only records. Retail PopoPOPO_SPECIAL_S_1maps to DAT index 314, whose animation size is zero but whose relocated script has 75 commands and 25 hitbox events;action_for_state()therefore returnsNoneduring Squall Hammer and the facade reports no attack or frame timing. Please reject only records lacking both animation and script after inheritance resolution, and add a script-only runtime-state regression test.@ -41,2 +43,3 @@"""def __init__(self, write=False):def __init__(self, write=False, *, iso_path=None):warnings.warn([P2] This unconditional warning is also reached through unchanged public APIs that internally construct
FrameData, includingget_framedata(),CharacterState, andSimpleControls. With the commonDeprecationWarning-as-error policy, those APIs now raise before doing any work even though their callers never selected the deprecated class and cannot substituteDiscFrameDatathere. Please migrate or suppress the internal construction paths while retaining the warning for direct deprecated use.@ -109,0 +134,4 @@"ISO-backed hitbox timing is unavailable for special states without fighter hitboxes; ""the move may create an unparsed article or projectile")return tuple(frame.local_frame for frame in record.timeline.frames if frame.active_hitboxes)[P2] These snapshots include script-declared hitboxes whose creation is runtime-conditional. Retail Fox
DAMAGE_FLY_HIGHhas only arequires_thrown_hitbox_ownergeneration, which the engine skips after an ordinary hit, but the facade reports an attack active on frames 1-28. Because this query has no owner context, please exclude conditional generations or report the timing as unavailable, and cover this state.Review comment #807: Fixed in
1bd7274. The compatibility facade now omits hitboxes requiring a runtime thrown-hitbox owner, while the lower-levelDiscFrameDatatimeline remains lossless and retains the conditional candidate. Added explicit low-level-retention and facade-filtering coverage. Focused tests:python -m unittest test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_nana_runtime_actions_inherit_missing_popo_animations(3 passed). Retail Fox DAMAGE_FLY_HIGH remains present in the low-level timeline but the facade now reports no attack and preserves its 28-frame duration. Full suite:python test.py(336 passed);py_compilepassed.Review comment #806: Fixed in
3a6eb11. After Nana inheritance resolution,action_for_state()now treats a record as empty only when it has neither animation data nor a relocated subaction script. Added synthetic script-only runtime mapping coverage and corrected the documented contract. Focused tests:python -m unittest test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_nana_runtime_actions_inherit_missing_popo_animations(2 passed). Retail Popo POPO_SPECIAL_S_1 now returns DAT 314 with 75 commands, 66 frames, and 25 hitbox events, while Fox NEUTRAL_ATTACK_3 remainsNone. Full suite:python test.py(336 passed); PR Ruff target set andpy_compilepassed.Review comment #805: Fixed in
25c04f7. The interpreter now keeps the signed command timer separate from elapsed animation time and preserves negative carry across later synchronous waits; the cycle state includes that timer. Added an overdue-absolute-then-relative timer regression. Focused tests:python -m unittest test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths(2 passed). Retail verification now reports Yoshi DAIR IASA at frame 53 and Samus BAT_SWING_3 hitboxes on frames 13-15. Full suite:python test.py(336 passed); PR Ruff target set andpy_compilepassed.Review comment #808: Fixed in
6eff0a4. Internal CSV compatibility construction inget_framedata(),CharacterState, andSimpleControlsnow suppresses only theFrameDataconstructor warning through a private switch; direct deprecated construction still warns. Added warnings-as-errors coverage for all three public paths. Focused tests:python -m unittest test.SLPFile.test_internal_framedata_construction_does_not_warn test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade(2 passed), plus a fresh-processpython -W error::DeprecationWarningprobe covering all three APIs. Full suite:python test.py(336 passed);py_compilepassed.Found four actionable correctness and contract issues in the ISO-backed framedata implementation.
@ -7,0 +62,4 @@retains those script-declared candidates and their condition flag.As with ``PlayerState.action_frame``, every exported ``local_frame`` and``FrameSnapshot.local_frame`` is one-indexed. ``ActionRecord.frame(1)`` always[P3] This
alwaysguarantee is false for public empty DAT records:DiscFrameData.action("Fx", 48)returns anActionRecordwhose timeline has no snapshots, soframe(1)raisesIndexError. Please either give empty records an initial snapshot or qualify the contract to records with a non-empty timeline and cover that behavior.@ -0,0 +137,4 @@header = stream.read(0x430)if len(header) != 0x430:raise DiscImageError("could not read the complete GameCube disc header")if header[0:6] != b"GALE01":[P2] This accepts any image that retains the
GALE01revision-2 header as canonical NTSC 1.02, after whichDiscFrameDatareports the pinned doldecomp provenance and reads fixed retail addresses. The PR's synthetic DOL, and realistic modded images, pass these checks and can return non-retail mappings despite the documented requirement not to apply NTSC 1.02 knowledge to modded files. Please verifymain.dolagainst the pinned retail fingerprint before exposing build metadata or runtime mappings, and test rejection of a same-header modified DOL.@ -0,0 +280,4 @@def _animation_snapshot_count(frame_count: float) -> int:if frame_count <= 0:return 0return max(1, math.ceil(frame_count) - 1)[P2] This always removes the FigaTree endpoint even when a reached state-changing command executes there. Retail Bowser action 362 has
animation_frame_count == 32and creates its hitbox at source/public frame 32; the event is retained but snapshots stop at 31, so the ISO facade reportsis_attack() == Falseand no hitbox instead of the frame-32 hitbox present in the checked-in capture. Please preserve an endpoint snapshot when a reached event occurs there, or report the timing as unavailable, and add this action as a regression test.@ -109,0 +131,4 @@if record is None:return ()if not record.timeline.hitbox_generations and self.is_bmove(character, action):raise DiscFrameDataError([P2] This intentional exception also escapes through
SimpleControlsbefore its existing special-action fallback. Continuing FoxNEUTRAL_BwhenPlayerState.actionisLASER_GUN_PULLwithFrameData(iso_path=...)callsis_attack()here and raises instead of returningAttackFrameData, so the new facade cannot be used with the public controls API for article specials. Please classify known specials before this hitbox query, or normalize unavailable timing insideSimpleControls, and add an end-to-end continuation test.Review comment #846: Fixed in
d01b477.GameCubeDisc.read_dol()now verifies the canonical NTSC 1.02main.dolSHA-1, andDiscFrameDataforces that check before exposing build provenance or fixed-address mappings. Added same-header modified-DOL rejection coverage plus private synthetic-fixture digest overrides. Tests:python -m unittest test.DiscFrameDataTests(10 passed on the final branch),python test.py(339 passed), PR Ruff target set andpy_compilepassed.Review comment #847: Fixed in
243071b. Exclusive FigaTree endpoints remain omitted for ordinary scripts, but reached hitbox or IASA events now extend snapshots through their actual frame. Added a regression proving an ordinary 8.0 endpoint stays at 7 snapshots while a frame-8 hitbox/IASA event produces the required eighth snapshot. Tests: focusedDiscFrameDataTestschecks passed,python test.py(339 passed), PR Ruff target set andpy_compilepassed.Review comment #848: Fixed in
c1b12bf.SimpleControlsnow recognizes a requested known special action before consulting hitbox timing, while direct ISO-backedFrameData.is_attack()calls still raise for unavailable article/projectile timing. Added an end-to-end Fox neutral-B continuation regression that returnsAttackFrameDataforLASER_GUN_PULL. Tests: focused controls run (64 passed),DiscFrameDataTests(10 passed),python test.py(339 passed), andpy_compilepassed.Review comment #849: Fixed in
243071b. Empty low-level DAT records remain empty rather than fabricating a frame, and the documentation now limitsframe(1)to non-empty timelines and explicitly documentsframe_count == 0/IndexErrorbehavior. Added assertions for empty Fox DAT index 48. Tests:DiscFrameDataTests(10 passed),python test.py(339 passed), strict Sphinx HTML build with warnings as errors passed.Found five actionable parser and framedata correctness issues.
@ -0,0 +155,4 @@f"FST size 0x{self.fst_size:X} exceeds the header maximum 0x{fst_max_size:X}")stream.seek(self.fst_offset)fst = stream.read(self.fst_size)[P2] This read, the analogous DOL read at line 286, and
read_member()materialize sizes controlled by unauthenticated image metadata. The current checks bound them only to the supplied file length, whilefst_max_sizecomes from the same header, so a standard 1.46 GB sparse ISO can request almost 1.46 GB in one allocation before structural parsing or DOL fingerprinting and terminate the process instead of raisingDiscImageError. Please enforce audited upper bounds for FST, DOL, and member sizes before reading them and add oversized-but-in-bounds sparse-image tests.@ -0,0 +91,4 @@def _validate_reference_chain(self, offset: int, index: int) -> None:seen = set()while offset != 0xFFFFFFFF:[P3] HSD external-reference links may terminate with either
0or0xFFFFFFFF; the pinned HSDRaw implementation explicitly stops on both. This loop follows a zero terminator into data offset zero, so a valid zero-terminated reference chain is rejected as cyclic. Please stop after reading either terminal link value and add a minimal zero-terminated reference regression test.@ -0,0 +738,4 @@frame_count = max(frame_count,max((event.local_frame for event in events), default=0),_frame(iasa_time) if iasa_time is not None else 0,[P2] This lets an IASA marker after a finite animation endpoint extend the action with unreachable snapshots. In the retail ISO, Fox f-smash ends at source time 40 but its script's stale IASA is at 46, so this now makes
timeline.frame_countand the publicFrameData.frame_count()/iasa()return 46 and exposes frames 41-46. The pinned runtime dispatches commands beforeanim_cb, which permits the endpoint fix, butftCo_AttackS4_Animtransitions as soon as the animation has no frames remaining, so time 46 is never reached (DK fair is similarly inflated from endpoint 60 to 85). Please only let IASA extend a FigaTree-bounded timeline when it occurs at the reachable endpoint, while retaining later raw script timing, and add a post-endpoint IASA regression test.@ -109,0 +130,4 @@record = self._disc_action(character, action)if record is None:return ()if not record.timeline.hitbox_generations and self.is_bmove(character, action):[P2] This uses legacy
is_bmove(), which treats nearly every action ID at or above 341 as a special even when the character-relative runtime state is not. Retail DK action 341 isHeavyWaitwith move IDThrowFand no fighter hitboxes, yetFrameData.is_attack()andCharacterState.get_state()raise the article-timing error instead of reporting a non-attack state. Please gate article uncertainty using character-relative MotionState special metadata rather than the numeric heuristic and cover DKHeavyWaitthrough the public state classifier.@ -629,0 +683,4 @@count = 0previous = Nonefor frame in frames:if previous is None or frame != previous + 1:[P2] This counts gaps in active frames, but parsed generations can replace hitboxes without an inactive frame. Retail Pichu
FSMASH_MIDcreates three hitbox IDs at each of six start frames (16, 19, 22, 25, 28, 31) and stays continuously active through frame 33, soFrameData.hitbox_count()returns1instead of6. Please count distinct unconditional generation start frames, grouping IDs created on the same frame, and add this forward-smash regression test.Review comment #910: Fixed in
37c0254. Added canonical NTSC 1.02 ceilings for FST, DOL, and supported fighter DAT/AJ member reads, checked after range validation and before allocation. Added oversized-but-in-bounds sparse image coverage for all three reads. Tests:python -m unittest test.DiscFrameDataTests.test_iso_validation_fst_bounded_read_and_exact_pairing test.DiscFrameDataTests.test_modified_dol_is_rejected_before_build_provenance(2 passed);uvx ruff check melee/_gamecube.pypassed;python test.py(341 passed).Review comment #911: Fixed in
e8ca05f. Reference validation now accepts a zero link after validating the current node, while retaining separate two-node cycle detection so data offset zero remains a valid chain head. Tests:python -m unittest test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context(1 passed);uvx ruff check melee/_hsd_dat.pypassed;python test.py(341 passed).Review comment #912: Fixed in
dd05276. Finite FigaTree timelines now extend for IASA only when the marker is reachable at or before the endpoint; raw post-endpoint IASA commands and timing remain retained. The endpoint hitbox/IASA regression still reaches frame 8, while a raw frame-14 IASA leaves an 8.0 endpoint at 7 snapshots. Tests:python -m unittest test.DiscFrameDataTests.test_reached_state_changes_extend_exclusive_animation_endpoint(1 passed); retail ISO verification reports Fox f-smash frame count/IASA 39/39 with raw IASA 46 and DK fair 59/59 with raw IASA 85;uvx ruff check melee/_subaction.pypassed;python test.py(341 passed).Review comment #913: Fixed in
7591d76. ISO article uncertainty now uses the audited MotionState move ID range instead of legacy numericis_bmove()classification; the legacy public heuristic remains unchanged. Added synthetic DK HeavyWait coverage throughCharacterState. Tests:python -m unittest test.DiscFrameDataTests.test_iso_article_uncertainty_uses_motion_move_id test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_iso_framedata_simple_controls_continues_article_special(3 passed); retail DK action 341 now reports move ID 53,is_attack() == False, andStanding;python test.py(341 passed).Review comment #914: Fixed in
3247b44. ISO hitbox counts now use distinct unconditional generation start frames, grouping IDs created on the same frame and excluding thrown-owner-conditional generations, instead of counting gaps in active snapshots. Added a continuously active six-generation, three-ID regression. Tests:python -m unittest test.DiscFrameDataTests.test_iso_hitbox_count_groups_generation_start_frames test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade(2 passed);python -m unittest test.DiscFrameDataTests(12 passed); retail Pichu f-smash now reports starts 16/19/22/25/28/31 and count 6;python test.py(341 passed).Found three actionable resource-safety and timeline correctness issues.
@ -0,0 +58,4 @@location = struct.unpack_from(">I", self._file, reloc_start + index * 4)[0]if location % 4 or location > self.data_size - 4:self._fail(f"relocation {index} has invalid data offset 0x{location:X}")if location in locations:[P2] This duplicate check linearly searches every prior relocation, making DAT validation quadratic in attacker-controlled
reloc_count. A 320,032-byte input with 40,000 unique relocations takes about 3.9 seconds here, while a member within the new 0x214D00 cap can encode roughly 272,000 entries and stall parsing for minutes. Please track seen locations in a set (which can also becomepointer_locations) and add a large unique-relocation regression.@ -0,0 +74,4 @@target, string_offset = struct.unpack_from(">II", self._file, roots_start + index * 8)if target >= self.data_size:self._fail(f"root {index} targets data offset 0x{target:X}")name = self._file_c_string(strings_start + string_offset, strings_start, f"root {index}")[P2] This independently decodes and retains every root name, so overlapping string-table offsets can amplify a bounded DAT far beyond its input size; the analogous reference-name path has the same issue. A 73,573-byte file with 1,000 offsets into successive suffixes of one 65,536-byte string peaks at about 65.2 MB, so a crafted member below the read cap can still exhaust process memory. Please bound aggregate root/reference name materialization relative to the member size, reuse identical decodes where possible, and cover overlapping offsets.
@ -0,0 +737,4 @@if animation_frame_count is not None:frame_count = max(frame_count,max((event.local_frame for event in events), default=0),[P2] This unconditional event maximum exports runtime frames past a non-looping animation endpoint. Retail Bowser
DOWN_B_GROUNDmaps to DAT action 313 with endpoint 40 and non-looping flags0x82000005, but its stale 21-damage create event at time 41 makes the facade report a fabricated frame-41 attack even though the animation callback transitions when frame 40 ends. Please retain post-endpoint events as raw script evidence but exclude them from snapshots and facade-visible generation counts for non-looping records, while passing the0x40000000loop flag so genuinely wrapping actions such as Game & Watch rapid jab remain unrestricted.Review comment #1003: Fixed in
4d6935c. Relocation locations are now accumulated in a set, making duplicate detection and the final pointer-location representation O(1) per entry. Added a regression that parses 40,000 unique relocations. Focused tests:python -m unittest test.DiscFrameDataTests.test_dat_metadata_resource_guards test.DiscFrameDataTests.test_dat_roots_actions_and_figatree_frame_count test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context(3 passed in 0.023s). Full suite:python test.py(342 passed);uvx ruff check melee/_hsd_dat.pyandpy_compilepassed.Review comment #1004: Fixed in
4d6935c. Root/reference names are cached by string-table offset, so repeated entries share one decoded string, and newly decoded name bytes have an aggregate budget capped at the DAT size. Added regressions for 1,000 overlapping suffix offsets and 1,000 repeated offsets. Focused tests:python -m unittest test.DiscFrameDataTests.test_dat_metadata_resource_guards test.DiscFrameDataTests.test_dat_roots_actions_and_figatree_frame_count test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context(3 passed in 0.023s). Full suite:python test.py(342 passed);uvx ruff check melee/_hsd_dat.pyandpy_compilepassed.Review comment #1005: Fixed in
a1ab986. Non-looping records now retain post-endpoint commands, hitbox events, and raw generations while excluding those events from snapshots and facade generation counts. The DAT0x40000000loop flag is passed through and exposed byActionRecord.animation_loops, so looping records remain unrestricted. Focused tests:python -m unittest test.DiscFrameDataTests.test_reached_state_changes_extend_exclusive_animation_endpoint test.DiscFrameDataTests.test_iso_hitbox_count_groups_generation_start_frames test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade(4 passed). Retail verification: BowserDOWN_B_GROUNDretains raw frame-41 damage 21 but reports(False, -1, -1, 0, 39)through the facade; Game & WatchLOOPING_ATTACK_MIDDLEretains frames through 63 and reports seven generations. Full suite:python test.py(342 passed); targeted Ruff andpy_compilepassed.Found five actionable parser and framedata correctness issues.
@ -0,0 +93,4 @@def _validate_reference_chain(self, offset: int, index: int) -> None:seen = set()while offset != 0xFFFFFFFF:[P2] This starts a fresh complete external-reference chain walk for every reference, while the per-call
seenset neither reuses validated suffixes nor rejects nodes already owned by another reference. A 70,922-byte DAT with 4,000 unique references to one 4,000-link chain passes validation after 16,000,000u32reads and takes about 4 seconds; the 0x214D00 member cap still permits roughly 37.2 billion visits. Shared chain nodes are invalid because each slot is resolved in place, but malformed DAT validation must remain bounded. Please track chain-location ownership globally, reject overlaps, and add a many-reference regression.@ -0,0 +207,4 @@result = []for index, offset in enumerate(range(actions, boundary, 0x18)):symbol_pointer = self.pointer(offset, description=f"action {index} symbol")symbol = self.c_string(symbol_pointer, f"action {index} symbol") if symbol_pointer is not None else None[P2] Action symbols use uncached
c_string(), so the root/reference-name cache does not cover them. A fully validatedftDataKirbyDAT using the pinned 479-action count can point every action at one 128 KiB symbol: the 145,604-byte input creates 479 distinct retained strings and peaks at about 63.2 MB. At the 0x214D00 member cap, this layout retains about 1.04 GB of decoded symbol payload alone. Please cache data-block strings by offset, enforce an aggregate decoded-symbol budget, and add a shared long-symbol regression.@ -0,0 +462,4 @@breakif opcode in (1, 2):frame = float(command.parameter("frame"))timer = timer + frame if opcode == 1 else frame - time[P3] Opcode 2 subtracts cumulative script time, but the runtime subtracts
CommandInfo.frame_count(cur_anim_frame + x898_unk), which differs after a wrap when action flag 0x20000000 is clear. Retail Female WireframeRUNNING(Gl action 13, endpoint 20, flags 0x4000001e) reachesAsyncTimer(33)at elapsed time 21; this timestamps the following opcodes 54/43/goto at 33, while the wrapped animation phase is 1 and the runtime schedules them at 53. Ordinary goto-loop timing remains part of the documented effective-timestamp contract; onlySet Timer Animationis explicitly truncated. Please track runtime frame count separately, pass the accumulation flag, and add a wrap-before-async regression.@ -109,0 +144,4 @@"the move may create an unparsed article or projectile")return tuple(frame.local_frame[P2] This exposes script-local frames directly as normalized
PlayerState.action_framevalues, but that alignment is action-specific. Peach action 45 is markedzeroindex=True, soConsoleincrements its live frame; the checked-in capture has hitboxes on frames 3-4, while the retail ISO facade reports 2-3,frame_count()19 instead of 20,iasa()16 instead of 17, and frame 2 as ATTACKING. Exhaustive comparison finds eight retail fighter-hitbox actions whose complete ISO window is exactly one frame early, including Peach/Zelda grabs and Zelda neutral-B. Please carry runtime frame alignment throughaction_for_state()into timeline/facade frames and cover these action-specific offsets.@ -629,0 +695,4 @@return 0return len({generation.start_frame[P2] Counting every generation start as another hit treats property redefinitions as rehit windows. Retail Mario NAir creates IDs 0/1 in group 0 at time 3, recreates the same enabled IDs/group at time 7, and clears at 33; this returns 2 while the checked-in CSV facade returns 1. Runtime preserves history for enabled same-ID/same-group CREATEs, while disabled or regrouped slots copy history from any other active same-group slot and clear it only when none exists. Please count group-scoped victim-history epochs across all four slots, so Mario remains one hit while Pichu f-smash's clear-separated waves remain six, and cover both cases.
Review comment #1028: Fixed in
d424d1a. External-reference chain locations are now tracked across the whole DAT; a second reference that shares a resolved slot is rejected immediately, bounding aggregate traversal to the available chain locations. Added a 4,000-reference/4,000-link shared-chain regression. Focused tests:python -m unittest test.DiscFrameDataTests.test_dat_metadata_resource_guards test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_dat_roots_actions_and_figatree_frame_count(3 passed in 0.026s). Full suite:python test.py(343 passed); targeted Ruff andpy_compilepassed.Review comment #1029: Fixed in
d424d1a. DAT data-block strings are now cached by offset and charged to an aggregate budget capped at the DAT data size. Added a fully validated 479-action Kirby layout with one shared 128 KiB symbol and asserted every action reuses the same decoded object. Focused tests:python -m unittest test.DiscFrameDataTests.test_dat_metadata_resource_guards test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_dat_roots_actions_and_figatree_frame_count(3 passed in 0.026s). Full suite:python test.py(343 passed); targeted Ruff andpy_compilepassed.Review comment #1031: Fixed in
e34cfd7.hitbox_count()now replays reachable unconditional create/remove/clear events in command order and counts only when a group has no active slot history; raw parser generations remain lossless. Added same-group redefinition, clear-separated wave, cross-slot group-copy, and endpoint regressions. Focused tests:python -m unittest test.DiscFrameDataTests.test_iso_hitbox_count_tracks_group_victim_history_epochs test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade(2 passed in 0.017s). Retail counts are Mario NAir 1, Pichu f-smash 6, Fox fair 5, Zelda up-smash 12, and Game & Watch rapid jab 7. Full suite:python test.py(343 passed);py_compilepassed.Review comment #1030: Fixed in
56cdfeb. The deprecated ISO facade now applies the eight audited action-specific compatibility offsets while low-levelActionTimelinerecords remain one-indexed script time, and the distinction is documented. Focused tests:python -m unittest test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry(2 passed). Retail verification matches all eight shipped windows: Peach jab 2 is(3, 4, 20, 17)and Zelda neutral-B is(13, 28, 60, 60)for first/last/count/IASA. Full suite:python test.py(343 passed); strict Sphinx, targeted Ruff, andpy_compilepassed.Review comment #1032: Fixed in
cce6dbe. The interpreter now tracks cumulative elapsed time separately from wrapped animation phase and the0x20000000accumulated-frame carry used by asynchronous timers;DiscFrameDataforwards that flag. Added wrapped and accumulating timer regressions plus parser-wiring coverage. Focused tests:python -m unittest test.DiscFrameDataTests.test_wrapped_animation_frame_drives_async_timers test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_reached_state_changes_extend_exclusive_animation_endpoint test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_iso_hitbox_count_tracks_group_victim_history_epochs(5 passed in 0.018s). Retail Female Wireframe running now timestamps opcodes 54/43/goto at 53.0, while Game & Watch rapid jab remains seven waves through frame 63. Full suite:python test.py(343 passed); targeted Ruff andpy_compilepassed.Found five actionable correctness and resource-safety issues.
@ -0,0 +200,4 @@if not name or not name.isprintable() or "/" in name or "\\" in name or name in (".", ".."):raise DiscImageError(f"FST entry {index} has unsafe name {name!r}")parent_index, parent_end, parent_path = stack[-1]path = f"{parent_path}/{name}" if parent_path else name[P2] The FST-size cap does not bound retained paths: each nested entry stores the complete parent path, while entries may reuse one long name-table string. A valid 6,213-byte FST with 100 nested 5,000-byte names retains 25,254,950 path characters (about 25.8 MB traced) before DOL authentication, and layouts below the 0x7529 cap can exhaust memory. Please cap directory depth or aggregate path materialization, cache decoded names, and reject over-budget FSTs.
@ -0,0 +438,4 @@while True:if len(executed) >= max_commands:raise SubactionParseError(f"{context}: command guard exceeded {max_commands} executions")state = (pc, time, timer, tuple(calls), tuple((loop[0], loop[1]) for loop in loops))[P2] The command guard does not bound interpreter memory because each execution retains complete copies of the call and loop stacks in the visited-state sets. A 516-byte script with 64 nested loops and 5,000 inner iterations executes 5,128 commands but peaks at 50.7 MB traced; the allowed 100,000 executions can exhaust the process. Please use compact state identities or enforce an aggregate control-flow complexity budget, and cover deeply nested finite loops.
@ -0,0 +375,4 @@)animation_frame_counts[animation_key] = frame_counttimeline = (interpret_subaction([P2] The frame guard applies independently to each action, but this loop rebuilds and retains a timeline for every record even when records share identical script and animation metadata. Replaying 100 identical 1,000-frame scripts creates 100 distinct timelines and 100,000 retained snapshots; a valid 479-action fighter table can request about 4.8 million snapshots on its first query. Please cache timelines by script/frame/flag identity and enforce a per-fighter aggregate snapshot or command budget.
@ -109,0 +152,4 @@if (not record.timeline.hitbox_generationsand state is not Noneand state.move_id in SPECIAL_MOVE_IDS[P2] Restricting unsupported-article detection to
SPECIAL_MOVE_IDSsilently misclassifies article attacks outside the four canonical special slots. On the retail ISO, PeachPARASOL_FALLINGhas move ID 87 and no fighter generations, so the ISO facade returnsis_attack() == False, hitbox frames-1/-1, andCharacterState.InAir, while the checked-in CSV reports an active attack on frames 1-100. Please include known character-owned article attack states in uncertainty classification and raiseDiscFrameDataErroruntil their timelines are parsed.@ -629,0 +722,4 @@if event.hitbox.requires_thrown_hitbox_owner:continuegroup = event.hitbox.hit_groupif group not in active_groups:[P2] This increments before considering subsequent events at the same timestamp, so zero-duration generations count as hits even though they never appear in a collision snapshot. Retail Yoshi
DAIRcreates IDs 0/1 and then clears all at time 46; frame 46 has no active hitboxes and the checked-in CSV reports 14 hits on frames 18 through 44, but the ISO facade returns 15. Please coalesce same-time events and count only victim-history epochs that survive into a collision-visible snapshot.Review comment #1059: Fixed in
ad33734. FST names are cached by string-table offset, and every candidate path is charged against the canonical image aggregate of 0x3D38 characters before construction. Added a structurally valid 6,213-byte, 100-level shared-name FST regression that verifies rejection and one name decode. Focused tests:python -m unittest test.DiscFrameDataTests.test_fst_name_and_path_materialization_is_bounded test.DiscFrameDataTests.test_iso_validation_fst_bounded_read_and_exact_pairing(2 passed in 0.004s). Full suite:python test.py(345 passed); targeted Ruff,py_compile, and strict Sphinx passed.Review comment #1060: Fixed in
635efa8. Call and loop snapshots are now created once per execution, shared by both visited-state sets, and charged against a 100,000 retained-stack-value aggregate guard while preserving cycle precedence and backward tail jumps. Added the 64-level, 5,000-iteration finite-loop regression. Focused tests:python -m unittest test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_wrapped_animation_frame_drives_async_timers(3 passed in 0.019s). Full suite:python test.py(345 passed); targeted Ruff andpy_compilepassed.Review comment #1061: Fixed in
8d4d3e5._parse_fighter()now shares immutable timelines by script offset, frame count, loop flag, and accumulation flag, and enforces a 110,000-item aggregate command-plus-snapshot budget across unique timelines. Tests cover shared timelines, behaviorally distinct keys, and aggregate rejection. Focused tests:python -m unittest test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry test.DiscFrameDataTests.test_nana_runtime_actions_inherit_missing_popo_animations test.DiscFrameDataTests.test_wrapped_animation_frame_drives_async_timers test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context(4 passed in 0.027s). Retail audit parsed 33 fighters and 10,658 actions; Kirby is the maximum at 23,601 retained items. Full suite:python test.py(345 passed); targeted Ruff andpy_compilepassed.Review comment #1062: Fixed in
e509ed9. Added the audited(PEACH, PARASOL_FALLING)article-attack exception without broadening move ID 87 or reintroducing the DK HeavyWait false positive. ISO hitbox queries andCharacterStatenow raise the documented unavailable-article error whileframe_count()remains available. Focused tests:python -m unittest test.DiscFrameDataTests.test_iso_article_uncertainty_includes_character_owned_attack_states test.DiscFrameDataTests.test_iso_article_uncertainty_uses_motion_move_id test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade(3 passed in 0.013s). Retail verification reportsDiscFrameDataErrorfor all four hitbox queries and state classification. Full suite:python test.py(345 passed);py_compileand strict Sphinx passed.Review comment #1063: Fixed in
ceda046.hitbox_count()now replays exact-animation-time batches and counts only newly started victim-history groups still active after the batch. Added a same-time create/create/clear regression and retained separated cross-slot, Mario, Pichu, endpoint, looping, and conditional coverage. Focused tests:python -m unittest test.DiscFrameDataTests.test_iso_hitbox_count_tracks_group_victim_history_epochs test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry(3 passed in 0.021s). Retail counts are Yoshi DAIR 14, Mario NAir 1, Pichu f-smash 6, Fox fair 5, Zelda up-smash 12, and Game & Watch rapid jab 7. Full suite:python test.py(345 passed); targeted static checks passed.Found five actionable correctness and parser-integrity issues.
@ -0,0 +278,4 @@f"disc member {entry.path!r} size 0x{entry.size:X} exceeds the supported maximum "f"0x{self._MAX_MEMBER_SIZE:X}")with self.path.open("rb") as stream:[P2]
DiscFrameDataauthenticates the DOL during construction, but lazy fighter reads reopen this path without verifying that it is still the same image. Replacing the ISO after construction let a probe retain the cached canonical DOL/build metadata whileaction("Fx", 0)read replacement DAT flags0xB0000042. Please bind all reads to one immutable image snapshot or detect mutation and reauthenticate before lazy member reads, and cover replacement between construction and first fighter query.@ -0,0 +92,4 @@references.append(DatRoot(name, target, True))self.roots = tuple(roots)self.references = tuple(references)self.object_offsets = tuple(sorted(targets | {self.data_size}))[P2] These structure boundaries omit every validated external-reference chain location, so a reference can begin inside the fighter root, action table, dynamic table, or a string without triggering the overlap checks. A synthetic DAT with a valid reference at offset
0x20inside the0x60fighter root still returned all 327 actions, and0x20was absent fromobject_offsets. Please mergereference_locationsinto the object boundaries and reject references overlapping required fighter structures.@ -0,0 +539,4 @@if opcode == 6:if not calls:raise SubactionParseError(f"{context}: return at DAT offset 0x{pc + 0x20:X} has no caller")pc = calls.pop()[P2] Separate call and loop stacks let a return cross a loop frame pushed after the call. A
call -> SetLoop -> returnstream currently returns to the caller and exposes its IASA command ([5, 3, 6, 23, 0]), but the runtime stores loop and call entries on one LIFO return stack, so that return consumes the intervening loop entry instead. Please model a unified typed control stack or reject operations whose top entry has the wrong type, and test both crossed call/loop forms.@ -109,0 +145,4 @@return self._disc_framedata.action_for_state(character, action)def _disc_frame_offset(self, character, action):return int((character, action) in _ISO_COMPATIBILITY_FRAME_OFFSETS)[P2] This boolean offset can express only zero or
+1, but two retail actions need a hitbox-only-1alignment. ISO-backed SamusFAIRreports active frames 5-6 through 31-32 instead of CSV frames 4-5 through 30-31, and MewtwoLOOPING_ATTACK_MIDDLEreports 6,13,...,48 instead of 5,12,...,47; both frame counts already correctly match at 55 and 50. Please add signed hitbox-specific alignment without subtracting from frame count, and cover the complete active-frame sequences.@ -629,0 +721,4 @@count = 0for animation_time, events in groupby(record.timeline.hitbox_events,key=lambda event: event.animation_time,[P2] Batching by raw animation time still counts epochs that disappear within one exported collision frame. A create at time 0 followed by
Wait(1)and clear at time 1 maps both events to local frame 1, whose snapshot has no active hitbox, yethitbox_count()returns 1. Retail KirbyNAIR_LANDINGandUAIR_LANDINGconsequently reportis_attack() == Falseand hitbox frames-1/-1but count 1. Please coalesce reachable events bylocal_frameas snapshot construction does and test this boundary.Review comment #1090: Fixed in
78622a5.GameCubeDiscnow captures the opened image identity fromfstat()while validating the header/FST and checks that identity before every lazy member or DOL read, so replacing the path cannot combine cached canonical metadata with another image. Added an atomic-replacement regression before the first fighter query. Tests:python -m unittest test.DiscFrameDataTests.test_disc_framedata_rejects_replaced_image_before_lazy_read test.DiscFrameDataTests.test_iso_validation_fst_bounded_read_and_exact_pairing test.DiscFrameDataTests.test_modified_dol_is_rejected_before_build_provenance(3 passed); fullpython test.py(346 passed); targeted Ruff andpy_compilepassed.Review comment #1091: Fixed in
d81a78f. Validated external-reference chain locations now participate inHsdDat.object_offsetswithout being treated as ordinary relocation pointers, so references embedded in the fighter root, action table, dynamic table, or strings are rejected by the existing overlap checks. Added an embedded fighter-root reference regression and updated the zero-terminated chain boundary assertion. Tests:python -m unittest test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_dat_roots_actions_and_figatree_frame_count test.DiscFrameDataTests.test_dat_metadata_resource_guards(3 passed); fullpython test.py(346 passed); targeted Ruff andpy_compilepassed.Review comment #1092: Fixed in
2e8f747. Calls and loops now share one typed LIFO control stack, preserving separate depth/resource guards while rejectingreturnacross an active loop andexecute-loopacross an active call instead of fabricating a valid timeline. Added both crossed-nesting regressions. Tests:python -m unittest test.DiscFrameDataTests.test_malformed_iso_dat_and_subactions_fail_with_context test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_wrapped_animation_frame_drives_async_timers(3 passed); fullpython test.py(346 passed); targeted Ruff andpy_compilepassed.Review comment #1093: Fixed in
f858acf. Added signed hitbox-only compatibility offsets for Samus FAIR and Mewtwo LOOPING_ATTACK_MIDDLE, leaving their already-correct frame counts and IASA values unchanged. Synthetic coverage asserts each complete corrected active-frame sequence. Tests:python -m unittest test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facadepassed; fullpython test.py(346 passed);py_compileand strict Sphinx passed. Canonical retail ISO probe now reports Samus(4,5,11,12,18,19,25,26,30,31)with frame count 55/IASA 50 and Mewtwo(5,12,19,26,33,40,47)with frame count 50/IASA 50.Review comment #1094: Fixed in
59c30b9. Hitbox victim-history epochs are now coalesced by exportedlocal_frame, matching snapshot construction, while each event retains its raw animation-time endpoint check. The regression uses create events at time 0 and a clear at time 1 that all map to local frame 1 and verifies a zero count. Tests:python -m unittest test.DiscFrameDataTests.test_iso_hitbox_count_tracks_group_victim_history_epochs test.DiscFrameDataTests.test_deprecated_framedata_iso_timing_facade test.DiscFrameDataTests.test_public_api_combat_timeline_and_local_geometry(3 passed); fullpython test.py(346 passed);py_compileand strict Sphinx passed. Canonical retail ISO now reports both Kirby NAIR_LANDING and UAIR_LANDING as non-attacks with-1/-1hitbox frames and count 0.Found five actionable issues in the ISO-backed framedata implementation.
@ -0,0 +130,4 @@_MAX_MEMBER_SIZE = 0x214D00def __init__(self, path: str | os.PathLike[str]):self.path = Path(path)[P3] This retains a relative path even though fighter members are reopened lazily. Constructing
DiscFrameData("game.iso")in the ISO directory succeeds, but after changing the process working directory its first fighter query raisesDiscImageErrorbecause the reopen resolves against the new directory, despitebuild.iso_pathidentifying the original absolute file. Please bind the stored path to its absolute construction-time location and cover a cwd change before the first lazy read.@ -0,0 +203,4 @@except OSError as exc:stream.close()raise DiscImageError(f"cannot stat disc image {self.path!s}: {exc}") from excif self._identity(metadata) != self._image_identity:[P3] This identity comparison occurs only before the subsequent seek/read, so an in-place writer can change the same inode immediately afterward and the altered member is accepted and cached under the original canonical build provenance. A synthetic probe changed
PlFxAJ.datafter thisfstat()and the lazy query returned the modified frame count without raising. Please compare descriptor identity again after each bounded read, before parsing or returning the bytes, and cover mutation between the pre-read check and read.@ -0,0 +199,4 @@self._fail("fighter action table is null")if actions % 4:self._fail(f"fighter action table has unaligned data offset 0x{actions:X}")if self.next_object_offset(fighter) < fighter + 0x60:[P2] Equal starts let incompatible required structures alias without tripping this greater-offset check. A DAT whose
ftDataroot and action-table pointer both target offset 0 passes the 0x60 root check and, with Fox's expected count, returns 327 fabricated actions; the action and dynamic tables can likewise share a start and pass their extent checks. Please explicitly reject equal or overlapping ranges among the typed fighter descriptor, expected action table, and dynamic table, while retaining valid general-purpose pointer aliases, and cover both equal-start cases.@ -0,0 +505,4 @@raise SubactionParseError(f"{context}: loop at DAT offset 0x{pc + 0x20:X} has zero count")if loop_depth >= max_loop_depth:raise SubactionParseError(f"{context}: loop depth guard exceeded {max_loop_depth}")control_stack.append((pc + 4, count))[P2] This push is guarded only by the independent loop-depth limit, so aggregate call/loop occupancy can exceed the five words available between the fighter
CommandInforeturn stack and the followingColorOverlay. The interpreter accepts three nested loops ([3, 3, 3, 4, 4, 4, 0]); the third runtime loop push writes indices 4 and 5, with index 5 landing atFighter+0x408inColorOverlay, yet parsing continues. Please bound each loop and call push socall_depth + 2 * loop_depth + pushed_words <= 5, and cover loop-only, call-only, and mixed overflow.@ -109,0 +160,4 @@assert disc_framedata is not Nonestate = disc_framedata.motion_state(character, action)if (not record.timeline.hitbox_generations[P2] This guard skips article uncertainty whenever an action also has fighter hitboxes, so mixed tether grabs silently return incomplete compatibility timing. On the canonical NTSC 1.02 ISO, Samus
GRABreports its first ISO-backed hitbox at frame 17 andWINDUPat frame 7, while the checked-in runtime capture includes its Grapple Beam marker at frame 7 and reportsATTACKING; Link and Young Link grabs have the same gap. Please raise for mixed article actions until their timelines can be merged, or include the article timing, and cover a tether grab.Review comment #1219: Fixed in
5cdbbf2. ISO-backed hitbox queries now fail closed for the six Link, Young Link, and Samus standing/running tether-grab states whose scripts contain both fighter and article hitboxes;hitbox_count()uses the same uncertainty guard, while non-hitbox timing remains available. Added focused synthetic coverage, updated the framedata documentation and AGENTS guidance, and verified all six pairs against the canonical NTSC 1.02 ISO. Verification: 5 focused tests passed, fullpython test.pypassed (353 tests), targeted Ruff andpy_compilepassed, and strict Sphinx completed with warnings as errors.Review comment #1220: Fixed in
5796edb. Fighter DAT validation now rejects pairwise-equal starts for the typed fighter root, action table, and dynamic behavior table before any range is decoded, while preserving valid duplicate targets in the generic HSD pointer table. Added regressions for each typed alias pair plus a control proving generic duplicate pointers remain accepted. Verification: 4 focused tests passed; the final fullpython test.pyrun passed (353 tests), targeted Ruff andpy_compilepassed.Review comment #1221: Fixed in
8eabee6. Runtime traversal now accounts for the actual five-wordevent_returnstorage: calls consume one word and loop frames consume two, so call-only, loop-only, and mixed scripts fail before a physical return-stack overflow. The existing configurable parser depth remains an additional resource bound. Added boundary and overflow regressions for all three shapes. Verification: 4 focused tests passed; the final fullpython test.pyrun passed (353 tests), targeted Ruff andpy_compilepassed.Review comment #1222: Fixed in
c1e14bc.GameCubeDiscnow resolves the supplied image path at construction, so lazy reads remain bound to the originally validated file even if the process changes working directory. Added a regression that constructs from a relative path, changes cwd, and successfully reads a member from the original image. Verification: 7 focused disc-read tests passed; the final fullpython test.pyrun passed (353 tests), targeted Ruff andpy_compilepassed.Review comment #1223: Fixed in
c1e14bc. Every lazy FST, DOL, and member read now revalidates image identity after consuming the bytes, closing the same-inode replacement window that existed after the pre-read check. Deterministic regressions mutate the image through an injected read callback and assert that each read path raisesDiscImageChangedError. Verification: 7 focused disc-read tests passed; the final fullpython test.pyrun passed (353 tests), targeted Ruff andpy_compilepassed.Found two actionable correctness and parser-integrity issues.
@ -0,0 +96,4 @@def _validate_reference_chain(self, offset: int, index: int, locations: set[int]) -> None:seen = set()while offset != 0xFFFFFFFF:[P3] This loop treats an initial
0xFFFFFFFFreference-table target as an already-terminated chain, so no bounds check runs andHsdDataccepts a 4-byte DAT while exposingDatRoot(data_offset=4294967295). The sentinel terminates a link read from a valid in-data chain node; it is not itself a valid chain head. Please validate the table target as an aligned in-data offset before recognizing terminators and cover a sentinel-head input.@ -0,0 +547,4 @@target_state = (target, control_snapshot)if opcode == 7 and target_state in seen_control_flow:script_loop = Truebreak[P2] Breaking at the first repeated
gotosilently omits iterations that still execute within the current animation cycle. On the canonical ISO, MewtwoNEUTRAL_B_CHARGINGcreates and clears its hitbox at times 0 and 3, jumps back at 5 in its 8-frame cycle, and therefore creates again before the cycle ends; the ISO facade reportslast_hitbox_frame() == 2andhitbox_count() == 1, while the checked-in runtime capture reports frame 6 and two hits. Please expand reachable loop iterations through the cycle boundary, or reject facade timing whenscript_loop_encounteredmakes it incomplete, and add this retail regression.Review comment #1244: Fixed in
315f93e. Repeatedgotocontrol flow now expands only when it advances time toward a known finite animation endpoint, then stops at that initial-cycle boundary; zero-progress and unknown-duration loops keep the safe first-pass behavior, and countedSetLoopscripts remain unchanged. Added parser/facade coverage including Mewtwo alignment. Verification: 5 focused tests passed, fullpython test.pypassed (354 tests), targeted Ruff andpy_compilepassed, strict Sphinx passed, and the canonical ISO audit parsed 33 fighters/10,658 actions with Mewtwo reporting first/last/count1/6/2and Game & Watch rapid jab still reporting 7 hits.Review comment #1245: Fixed in
afbccc4. External-reference validation now requires the table head itself to be an aligned in-data node before accepting either0or0xFFFFFFFFread from that node as the chain terminator. Added a malformed sentinel-head regression while preserving both valid terminator forms. Verification: 2 focused DAT parser tests passed; final fullpython test.pypassed (354 tests), targeted Ruff andpy_compilepassed.Found two actionable correctness and availability defects.
@ -0,0 +132,4 @@def __init__(self, path: str | os.PathLike[str]):self.path = Path(path).resolve()try:stream = self.path.open("rb")[P2] This opens the user-supplied path before checking
S_ISREG, so on POSIX a FIFO with no writer blocks forever instead of raisingDiscImageError; the same ordering in_open_validated_image()lets a FIFO swapped in before a lazy read hang before identity validation. Anos.mkfifo()probe leftGameCubeDisc(path)blocked until externally terminated. Please open nonblocking,fstat()the descriptor before wrapping it as a stream, and add POSIX-gated tests for initial and replacement FIFOs.@ -0,0 +490,4 @@if (expanding_script_loopand animation_frame_count is not Noneand next_time >= animation_frame_count[P2] This new loop-expansion cutoff also fires when a wait lands exactly on the finite endpoint, before the following command can run. The runtime dispatches commands whose timer reaches zero before the animation callback; retail Mewtwo DAT actions 296/297/301/302 and Kirby actions 436/437/441/442 therefore execute
Clear Hitboxesat time 8, but these timelines omit that event and expose only seven snapshots. Please retain commands at equality, stop only when the wait overshoots the endpoint, and cover exact-endpoint command execution in a loop-expanded script.Review comment #1279: Fixed in
b1cd6d4. Initial and lazy disc-image opens now useO_NONBLOCKuntilfstat()confirms the descriptor is a regular file, so a user-supplied or replacement FIFO raisesDiscImageErrorwithout waiting for a writer. Added POSIX-gated, timeout-protected subprocess regressions for both paths. Verification: 5 focused disc identity/FIFO tests passed, targeted Ruff andpy_compilepassed, and finalpython test.pypassed all 368 tests.Review comment #1280: Fixed in
42665d5. Loop expansion now stops only when a wait overshoots the finite endpoint, allowing commands due exactly at equality to execute. The regression now retains the endpoint clear and eighth snapshot while preserving the two Mewtwo hit windows. Verification: 4 focused interpreter tests passed; the canonical ISO audit parsed 33 fighters/10,658 actions and all eight affected Mewtwo/Kirby records now end withClear Hitboxesat time 8; targeted Ruff,py_compile, strict Sphinx, and the full 368-test suite passed.Found two actionable correctness and portability issues.
@ -0,0 +181,4 @@file_descriptor = os.open(self.path, flags)except IsADirectoryError as exc:raise DiscImageError(f"disc image is not a regular file: {self.path!s}") from excexcept OSError as exc:[P2] On Windows, opening a directory with
os.openraisesPermissionError/WinError 5 rather thanIsADirectoryError, so the supported Windows path falls into this generic branch and the new unconditional directory test fails because it receivescannot openinstead ofregular file. Please classify directories in this error path while retaining descriptor-levelfstatvalidation for replacement races, and cover or emulate Windows' error mapping.@ -0,0 +557,4 @@if opcode == 7 and target_state in seen_control_flow:script_loop = Truerepeated_state = (target, time, timer, control_snapshot)if not animation_frame_count or time >= animation_frame_count or repeated_state in seen:[P2] This still stops before following a repeated
gotowhen that jump itself is reached exactly at the finite endpoint, so zero-time commands at its target are omitted even though command dispatch precedes the animation callback. On the canonical ISO, Kirby actions 412/420 and Jigglypuff actions 302/310 all reach an endpointgototoClear Hitboxes; their 60-frame timelines stop at 59 and omit the clear at time 60. Please follow the endpoint jump and stop at the first subsequent overshooting wait or repeated full state, and cover a target command reached by an endpointgoto.Review comment #1293: Fixed in
7da6e70. Repeated gotos reached exactly at the finite endpoint now follow their target before the existing overshooting-wait or repeated-state guards stop expansion. Added a regression whose endpoint goto reaches a zero-time hitbox command. Verification: 4 focused endpoint/control-flow tests passed; the canonical NTSC 1.02 audit parsed 33 fighters/10,658 actions and Kirby 412/420 plus Jigglypuff 302/310 now retainClear Hitboxesat time 60 with 60 snapshots; affected-source Ruff andpy_compile, strict Sphinx, and fullpython test.py(368 tests) passed.Review comment #1294: Fixed in
fb7cb7e. Windows-style directoryPermissionErroris now normalized to the regular-file diagnostic after confirming the path is a directory, while permission failures for regular files retain the generic open error and descriptor-levelfstatvalidation remains unchanged. Added portable regressions for both classifications. Verification: 4 focused disc/FIFO/identity tests passed; affected-source Ruff andpy_compile, strict Sphinx, canonical 33-fighter/10,658-action ISO audit, and fullpython test.py(368 tests) passed.Found two actionable portability and deployment issues.
@ -0,0 +130,4 @@_MAX_MEMBER_SIZE = 0x214D00def __init__(self, path: str | os.PathLike[str]):self.path = Path(path).resolve()[P3] Resolving the path before the guarded open leaks
RuntimeErrorfor a circular symlink on supported Python 3.10-3.12 (Path.resolve()only stopped raising for non-strict symlink loops in 3.13), soDiscFrameData(path)bypasses theDiscImageErrornormalization used for other malformed or unopenable image paths. This is reproducible on 3.11 and 3.12. Please normalize resolution failures, or bind the absolute path without eagerly resolving symlinks, and add a version-compatible circular-symlink regression.@ -0,0 +367,4 @@if len(data) != dol_size:raise DiscImageError("could not read the complete DOL")self._assert_image_unchanged(stream)digest = hashlib.sha1(data).hexdigest()[P2] This unconditional SHA-1 constructor is blocked on real FIPS-enforced deployments such as RHEL 9, where
hashlib.sha1(data)raisesUnsupportedDigestmodError/ValueError; becauseDiscFrameData.__init__()always reads the DOL, even the canonical ISO becomes unusable before any query. Please authenticate the DOL with a FIPS-supported digest such as SHA-256, update the pinned fingerprint and diagnostic, and cover a blocked-SHA-1 runtime.Review comment #1293: Reverified at current head
fb7cb7e; the fix remains in pushed commit7da6e70, so no additional code change was needed. The pre-fix parent omits the endpoint target, while current code follows the repeated goto at equality and retains the target command at time 60 for Kirby 412/420 and Jigglypuff 302/310. Tests:python -m unittest test.DiscFrameDataTests.test_repeated_goto_expands_to_animation_endpoint test.DiscFrameDataTests.test_reached_state_changes_extend_exclusive_animation_endpoint test.DiscFrameDataTests.test_control_flow_and_lossless_command_lengths test.DiscFrameDataTests.test_wrapped_animation_frame_drives_async_timers(4 passed); canonical ISO audit parsed the affected records correctly;python test.py(368 passed); affected-source Ruff,py_compile, and strict Sphinx passed.Review comment #1294: Reverified at current head
fb7cb7e; the fix remains in pushed commitfb7cb7e, so no additional code change was needed. Deterministic Windows-stylePermissionErroremulation now classifies directories as non-regular files while preservingcannot openfor denied regular files, and descriptor-levelfstatvalidation remains intact. Tests:python -m unittest test.DiscFrameDataTests.test_iso_validation_fst_bounded_read_and_exact_pairing test.DiscFrameDataTests.test_disc_image_rejects_fifos_without_blocking test.DiscFrameDataTests.test_disc_construction_rejects_in_place_change_after_identity_check test.DiscFrameDataTests.test_disc_member_read_rejects_in_place_change_after_identity_check test.DiscFrameDataTests.test_disc_dol_read_rejects_in_place_change_after_identity_check(5 passed);python test.py(368 passed); affected-source Ruff,py_compile, and strict Sphinx passed.View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.