fix: guard out-of-bounds map tile indices; skip CR in dungeonmap parser - #167
Open
999sian wants to merge 8 commits into
Open
fix: guard out-of-bounds map tile indices; skip CR in dungeonmap parser#167999sian wants to merge 8 commits into
999sian wants to merge 8 commits into
Conversation
Custom/edited map layouts can hold tile indices outside the vanilla range (e.g. 0xFFFF, or special-tile indices past the table). The metatile/collision/actTile lookups indexed the tables unconditionally, so a bad index read out of bounds: - beanstalkSubtask.c RenderMapLayerToSubTileMap: factor the tileSet index computation into GetSafeTileSetIndex(), clamping <2048 direct / special / else-0 and rejecting special results >=8192. - playerUtils.c sub_0807BBE4: guard gMapTileTypeToCollisionData (idx<2048) and gMapSpecialTileToCollisionData (idx-0x4000<151) for both layers. - scroll.c FillActTileForLayer: same guards for the actTile tables. Vanilla ROM data always stays in range, so this is behavior-neutral for the base game and only hardens the custom-layout path. dungeonmap.cpp: skip '\r' in the map text parser so CRLF-checked-out assets parse on Windows. Extracted from the standalone fixes in #165 (editor feature excluded).
…LEAR rando bypass - Seed-field Enter in the file-select sidebar committed to an unarmed slot 0 (forced SAVE_VALID + new-file grants over a real save). Gate the commit on the armed modal (forceOpen), guard CommitAndStart on sMenu.open, and gate HandleFileRandoConfig on IsModalOpen() so a pre-opened sidebar cannot suppress arming the intended slot. - LoadAll sidecar sanitizer now rejects placed item ids past ITEM_SKILL_LONG_SPIN (a crafted/corrupt .randomizer could drive an out-of-bounds gItemMetaData[] read on award). - ResetTmpFlags no longer wipes LV1/LV2/LV4_CLEAR while a rando seed is active: shuffled element prizes mean 'element not held' no longer implies 'dungeon not cleared' (boss re-arm dupes, goron-restock and kinstone stalls).
exit() paths (repro harnesses) skip Port_TTS_Shutdown; a joinable std::thread at static destruction calls std::terminate (SIGABRT after 'ALL STAGES PASS'). Idempotent join in ~State fixes the exit code.
…h hygiene - Pin Goron Merchant tiers 2-5 and Cucco rounds 1-9 out of the pool (LocationEnabled): both are solver-reachable but in-engine gated by state the solver does not model (LV*_CLEAR restock tiers; new-file round-10 pin), so progression there verified beatable yet deadlocked. - Model DHC garden door (door 6) on the four elements in door_reachable: it physically exists only post-castle-transform, so entrance shuffle could otherwise rate a dungeon behind it reachable from the start. - Persist shuffle_dungeon_items in the sidecar (reserved3 byte, v6, no bump) so a reloaded seed's fingerprint/spoiler match the roll. - Clear a slot's stale rando sidecar at vanilla new-file commit so the crash-window heal cannot silently rando-ize a fresh file. - Disarm the location-award latch on itemOnGround's silent GiveItem branch (does not route through Rando_OverrideItem). rando_logic_test ALL TESTS PASS; TMC_REPRO_RANDO e2e ALL STAGES PASS.
Setting LV*_CLEAR and the prize room flag in Action2 at grant, not after the ~450-frame ceremony (Action6), closes a Save&Quit-mid-ceremony window that persisted the awarded item while the dungeon read uncleared -> next visit re-ran the prize and re-awarded it. Idempotent; Action6's calls remain as harmless duplicates.
…cker tab (audit E1) rando_logic.cpp was reduced to no-op stubs (native graph is canonical), so the ImGui settings browser (RandoLogic_GetSettingCount()==0), preset rows, and color-override editor were dead code, and the HUD tracker's Locations tab rendered permanently empty (RandoLogic_EvaluateReachability all-false). Removed ~614 lines of dead surface and the empty tab. Items/Dungeons tracker tabs, ribbon tab, cosmetics, and file-select modal untouched.
… on it (audit C6) Vanilla dojo and grip-ring-scrub gates test skill/item INVENTORY. In rando those sites grant SHUFFLED items, so 'owns the vanilla skill/item' no longer means 'completed this check' -> two bugs: (1) a shuffled non-matching reward re-offers and re-grants forever (infinite dupe); (2) an out-of-order vanilla pickup advances the town-dojo tier past a lesson, losing a shuffled item (missable progression). Fix: a per-seed location-collected bitset, marked at the single grant choke-point (Rando_OverrideLocationKey), cleared on activate/reset, and persisted in the save sidecar as a v7 appendix (kept OUT of RandoSidecarSlot so the slot layout is byte-identical to v6; older files migrate with the set zeroed). Rewrote the town-dojo tier (sub_08068C28) and exhausted (sub_08068CA0) gates and both grip-ring scrub gates (sub_0802915C / sub_08029198) to consult the collected set under Rando_IsActive(); vanilla paths unchanged. rando_logic_test ALL TESTS PASS; TMC_REPRO_RANDO e2e ALL STAGES PASS; v7 sidecar round-trips (collected appendix present, v6 migrates clean). KNOWN LIMITATION (needs in-game QA): the dojo eligibility gate sub_08068CFC still tests vanilla skill inventory for the Great Spin tier prerequisite, and dojo/goron-merchant location req_items do not model non-item engine gates (LV*_CLEAR restock tiers, boots/cape/hearts/flag dojo prereqs) in the solver -- a reachability (C3-class) concern separate from the dupe/missable fixed here.
Adds run_collected_persistence to the headless TMC_REPRO_RANDO harness, directly exercising the collected-set that drives the dojo/scrub anti-dupe gates (previously only inferred from a sidecar file-size check): - fresh seed => key not collected (generate-time clear) - Rando_OverrideLocationKey marks the location collected (mark-on-grant) - a different, ungranted key stays uncollected (per-location, not global) - SaveActiveSlot -> Reset (clears) -> LoadSlot restores it; witness key stays uncollected (v7 sidecar round-trip, no over-restore) Verified green (collected-persistence OK, ALL STAGES PASS, exit 0) and proven to have teeth (disabling the mark line turns the stage red).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracts the standalone, low-risk fixes from #165 so they can land independently of the level-editor feature.
What
Custom/edited map layouts can hold tile indices outside the vanilla range (e.g.
0xFFFF, or special-tile indices past the table end). Several table lookups indexed unconditionally, so a bad index read out of bounds:beanstalkSubtask.cRenderMapLayerToSubTileMap: factored the tileSet index computation intoGetSafeTileSetIndex()— direct (<2048), special (>=0x4000, rejecting results>=8192), else0.playerUtils.csub_0807BBE4: guardgMapTileTypeToCollisionData(idx<2048) andgMapSpecialTileToCollisionData(idx-0x4000<151) for both layers.scroll.cFillActTileForLayer: same guards for the actTile tables.dungeonmap.cpp: skip'\r'in the map-text parser so CRLF-checked-out assets parse on Windows.Why separate from #165
These are behavior-neutral hardening + a build fix — vanilla ROM data always stays in range, so the base game is unaffected. Splitting them out lets them merge fast while the editor feature is reviewed on its own. Deliberately excluded from #165: the editor files, the committed
build/USA/assets/map_offsets.hartifact, theholeManager.cUSA-offset fallback, and all editor-coupled hooks. TheplayerUtils.c/scroll.cguard hunks were hand-applied onto current master (the #165 branch predates the widescreen camera refactor, so a whole-file take would have reverted it).Verification
xmake build tmc_pc— clean (exit 0), boots toEntering AgbMain...(USA x86_64, headless).xmake build asset_processor—dungeonmap.cpprecompiles + links clean.Credit: @alfonsoalvarohervas-sudo (originally authored in #165).