#6917 guarded the three seqLoadStatus accessors against sequenceMapSize
because the array was allocated at exactly that size, so a custom sequence
id past it read and wrote off the end.
#6932 sized the array to sequenceMapSize + 0xF to match sequenceMap, which
covers the whole id space rather than rejecting the ids outside it, and also
protects the writers those guards never saw: AudioHeap_AllocCached and
AudioHeap_PopCache index seqLoadStatus directly.
The guards are now not just redundant but bounded wrong - they treat the ids
in [sequenceMapSize, sequenceMapSize + 0xF) as absent from a table that now
has room for them. Nothing breaks today because every entry starts at 5, so
AudioLoad_IsSeqLoadComplete answers true regardless and
AudioLoad_SetSeqLoadStatus already declines to overwrite a 5 - but once the
heap cache path moves such an entry off 5, the guard blocks an update that
should happen.
Reverting them restores the three functions to their decompiled form. The
sizing from #6932 and the id check in AudioLoad_SyncInitSeqPlayerInternal
remain the actual protection.
Reaching the Fast3d GUI is spelled out in full at every call site:
std::dynamic_pointer_cast<Fast::Fast3dGui>(
Ship::Context::GetRawInstance()->GetWindow()->GetGui())->GetTextureByName(name)
Each one locks the Context weak_ptr, copies a shared_ptr for the window
and one for the GUI, then runs a dynamic_cast, only to drop all three
again. InputViewer::DrawElement does that 43 times per frame, and the
item tracker and save editor repeat it once per icon drawn.
Binding it to a local once per scope leaves the behaviour identical and
reads better. The local goes in the innermost block that covers the call
sites, so it stays behind the guards that were already there: DrawElement
still resolves nothing when the input viewer is off.
RenderButton keeps its inline casts. Its two call sites are in exclusive
branches, so a local would add a cast on the path that takes neither.
Warp song cutscenes were skipped for randomizer players whether they
wanted it or not: Demo_Kankyo cut the departure animation short from a
hardcoded IS_RANDO branch, and entrance rando cleared respawnFlag so the
arrival never played. There was no way to get the vanilla warp back, and
no way for a vanilla playthrough to skip it.
Drop both and put the behavior behind a Skip Warp Cutscenes toggle,
defaulting on in randomizer so existing rando warps are unchanged. It
kills the departure actor as it spawns and puts the arrival spawn mode
back to IDLE, which covers both halves of the warp.
Entrance rando's destination override moves off Demo_Kankyo's update onto
a new OnWarpSongLeave hook fired from Environment_WarpSongLeave. That is
where every warp path -- the cutscene, the skip toggle, and the spawn
failure fallback -- commits its destination, so the override no longer
depends on the departure actor being alive to see it, and a warp song
shuffled onto a grotto return keeps its grotto respawn.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Fix yaml-cpp inclusion for VS building.
* Fix double-finds for rom files in the binary directory.
* Fix archive generation targets.
* Port over Proxy's change on 2ship to old ROM deletion to search all applicable subdirectories.
A stored value with no entry in the combo map threw out of map::at while
drawing, which took the whole menu down as soon as a search matched the widget.
Fall back to the default and log the offender.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The source globs did not use CONFIGURE_DEPENDS, so adding a file under
soh/ or src/ left it out of the build until cmake was re-run by hand,
silently and with no warning.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fmt::format returns a std::string by value, so taking c_str() of the temporary
left BeginCombo reading freed memory whenever the melee weapon state fell
outside the known cases. Hold the text in a std::string instead.
Use spdlog::fmt_lib in the save editor
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Root cause of three identical field crashes (audio thread, opcode fetch
through a pointer with its low 32 bits overwritten, seconds after scene
transitions): AudioHeap_AllocPermanent writes permanentCache[index] with
index = permanentPool.count and no bound against the 32-entry array. In
SoH every soundfont sync-load is forced permanent, and custom sequences
whose SEQ.xml says CachePolicy="Temporary" ALSO allocate permanently
(the factory stores the LUS enum where CACHE_TEMPORARY == 0, while
AudioLoad_SyncLoad's switch reads 0 with the ROM convention
'permanent'). A pack with ~60 streamed customs plus vanilla fonts pushes
count past 32 within a session, after which each allocation sprays a
{ptr, size, tableType/id} triplet at 24-byte stride through
gAudioContext - entry[135]'s ptr field lands exactly on
seqPlayers[0].scriptState.pc and entry[156] on seqPlayers[1]'s (both
verified against the crash-dump registers).
- permanentCache raised 32 -> 512 (12 KB) and AllocPermanent refuses
allocations past the array instead of corrupting memory.
- Same unbounded-index disease fixed in the three sibling writers:
AllocCached's persistent path (16-entry array; CACHE_EITHER degrades
to temporary, hard persistent requests fail cleanly),
AllocPersistentSampleCacheEntry, AllocTemporarySampleCacheEntry.
- seqLoadStatus malloc sized for the full id space (sequenceMapSize +
0xF) matching sequenceMap; custom ids above sequenceMapSize previously
overflowed the allocation by up to 15 bytes.
Upstream SoH bugs, not branch-introduced - this branch's many-track
packs merely made the overflow reachable in normal play. Standalone
upstreamable fix.
djevangelia