fix(memory): Fix various memory leaks (2)

[Caball009]

This PR fixes 10 memory leaks (in order of commits):

0. Potential memory leaks, but I've not observed any actual leaks here.
1. BattlePlanUpdate::m_bonuses was never deallocated, so every time a USA Strategy Center was built, there'd be a leak.
2. Observed when looking at the Particle Cannon beam. SegLineRendererClass::Get_Texture has a side effect as it increases the reference count, so shouldn't be used here.
3. Player upgrades that were cancelled would leak. Can be reproduced by building a Barracks and starting the Capture Building upgrade and then cancelling it.
4. Scripts that would load on map start could leak. I have a map that generates more than 90K of leaks because of this.
5. Animation handle wasn't cleaned up even though that's expected. There's even a comment that says so... I think I observed a leak here when looking at a USA Strategy Center.
6. It was possible that the quit menu window would not be deallocated. Can be reproduced by loading a save game or replay. Make sure to load the quit menu at least once (by pressing ESC), and make sure that the game ends with a victory / defeat screen, so not manually leaving the game.
7. Pausing the game would leak audio events that were in the audio request container at the time.
8. The static DynamicAudioEventInfo and everything it would allocate would not be deallocated before the game process ends, and show up as a leak.
9. / 10. Loading a save game (skirmish or challenge) would allocate TheSkirmishGameInfo or TheChallengeGameInfo but not deallocate them.

TODO:

• Update this post and remove the commit for audio event leaks because that's extracted to fix(memory): Fix memory leak for audio events when pausing the game #2731
• Fix crash when restarting a challenge.
• Replicate in Generals.

[greptile-apps]

Greptile Summary

This PR fixes 10 memory leaks across the Zero Hour engine, covering animation channel loading, audio request lifecycle, script list management, upgrade cancellation, and save-game state objects. Most fixes are straightforward and correct; the TheSkirmishGameInfo/TheChallengeGameInfo deletion timing (already flagged in review threads) remains an open design question.

• Audio request lifecycle (AudioRequest.cpp, MilesAudioManager.cpp): The destructor now owns m_pendingEvent cleanup, while processRequestList resets the flag after handing ownership to processRequest, correctly preventing both the pause-path leak and double-free.
• Animation channel leaks (hcanim.cpp, hrawanim.cpp): Channel variables are now scoped to their switch cases and the allocated object is deleted when Load_W3D returns false, closing the leak on partial load failure.
• Miscellaneous fixes (BattlePlanUpdate, seglinerenderer, Player, SidesList, ScriptDialog, GameStateMap, W3DModelDraw, Drawable): Each targeted leak is addressed with appropriate deleteInstance/delete[] calls at the correct ownership-transfer points.

Confidence Score: 3/5

Most leak fixes are safe, but the unconditional deletion of TheSkirmishGameInfo/TheChallengeGameInfo inside clearGameData breaks the challenge campaign Next Mission flow and score-screen music, a crash path acknowledged in the review thread but not yet resolved.

The vast majority of the 10 fixes are clean and correct — animation channel scoping, audio request lifecycle, upgrade deletion, script list cleanup, and map buffer error paths all look solid. The one unresolved problem is in GameLogicDispatch.cpp: deleting TheSkirmishGameInfo/TheChallengeGameInfo unconditionally whenever showScoreScreen is true means the challenge Next Mission click crashes because ScoreScreen.cpp asserts TheChallengeGameInfo != nullptr before reusing it, and skirmish score-screen music is silently broken.

Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp — the game info deletion block needs a guard so it only runs when the info was originally allocated during a save-game load, not during a live campaign or skirmish session.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/Common/Audio/AudioRequest.cpp	Adds destructor to delete m_pendingEvent when m_usePendingEvent is true, fixing the leak when requests are discarded (e.g. on pause).
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp	Restructures clearGameData to add destroyQuitMenu and TheSkirmishGameInfo/TheChallengeGameInfo deletion; the campaign Next Mission crash on deletion of TheChallengeGameInfo is acknowledged but unresolved.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Resets m_usePendingEvent and m_pendingEvent to prevent double-free after processRequest takes ownership of the audio event.
Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp	Moves getAnimHandle() call (which AddRefs) inside the m_renderObject null guard, preventing a ref-count leak when m_renderObject is null.
Core/Libraries/Source/WWVegas/WW3D2/hcanim.cpp	Scopes tc_chan/ad_chan/newbitchan variables to their case blocks and deletes the channel object on Load_W3D failure, fixing leaks on partial animation load.
Core/Libraries/Source/WWVegas/WW3D2/seglinerenderer.cpp	Replaces Get_Texture() (which AddRefs) with Peek_Texture() in the debug warning path, preventing an unmatched reference count increase.
GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp	Adds deleteInstance(upgrade) inside the upgrade guard and moves markUIDirty() inside the same guard; correctly fixes the cancel-upgrade leak.
GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameStateMap.cpp	Adds delete[] buffer before each error-path throw in embedPristineMap, embedInUseMap, and extractAndSaveMap; all three sites have a non-null buffer at those points.
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp	Replaces the heap-allocated singleton DynamicAudioEventInfo marker with a static subclass instance (DynamicAudioEventInfoStatic), avoiding both the static-init-order fiasco and the leak.
GeneralsMD/Code/GameEngine/Source/GameLogic/Map/SidesList.cpp	Adds deleteInstance for scripts that cannot be matched to a skirmish side, preventing a leak during prepareForMP_or_Skirmish.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/BattlePlanUpdate.cpp	Adds deleteInstance(m_bonuses) in the destructor, fixing the per-Strategy-Center build leak.
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/hrawanim.cpp	Scopes newchan/newbitchan to their case blocks and correctly deletes the channel on Load_W3D failure, fixing leaks on partial animation load.
GeneralsMD/Code/Tools/WorldBuilder/src/ScriptDialog.cpp	Adds deleteInstance for scripts that cannot be matched to a player side or whose target side has no existing script list, preventing leaks during script import.

_{Reviews (4): Last reviewed commit: "Addressed feedback & made tweaks." | Re-trigger Greptile}

[greptile-apps]

TheChallengeGameInfo deletion breaks the challenge campaign "Next Mission" flow

clearGameData(TRUE) (which shows the score screen) now unconditionally deletes TheChallengeGameInfo. In ScoreScreen.cpp, startNextCampaignGame() immediately asserts TheChallengeGameInfo != nullptr (line 200) before re-configuring it for the next mission. Since clearGameData() runs before the score screen updates, TheChallengeGameInfo is already null by the time the player clicks "Next Mission", causing an assertion failure / crash on every challenge campaign mission transition.

The same applies to TheSkirmishGameInfo being set to null before ScoreScreenUpdate() uses TheGameInfo (line 428 of ScoreScreen.cpp) to start score-screen music, silently breaking score-screen music for all regular skirmish games.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when isLoadingSave() was true).

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp
Line: 283-294

Comment:
**`TheChallengeGameInfo` deletion breaks the challenge campaign "Next Mission" flow**

`clearGameData(TRUE)` (which shows the score screen) now unconditionally deletes `TheChallengeGameInfo`. In `ScoreScreen.cpp`, `startNextCampaignGame()` immediately asserts `TheChallengeGameInfo != nullptr` (line 200) before re-configuring it for the next mission. Since `clearGameData()` runs _before_ the score screen updates, `TheChallengeGameInfo` is already null by the time the player clicks "Next Mission", causing an assertion failure / crash on every challenge campaign mission transition.

The same applies to `TheSkirmishGameInfo` being set to null before `ScoreScreenUpdate()` uses `TheGameInfo` (line 428 of `ScoreScreen.cpp`) to start score-screen music, silently breaking score-screen music for all regular skirmish games.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when `isLoadingSave()` was true).

How can I resolve this? If you propose a fix, please make it concise.

[Caball009]

Good point.

A targeted fix would guard these deletions to only run when the game info was allocated during a save-game load (e.g. by checking an additional flag or by only deleting when isLoadingSave() was true).

This doesn't fix the crash that happens when restarting a challenge match from the 'score screen'. I could use some suggestions where these deallocations are better suited to take place.

[xezon]

I think it makes sense to split this into multiple reviews because of the review compexity of some.

(My review is incomplete)

[Caball009]

I think it makes sense to split this into multiple reviews because of the review compexity of some.

Are there particular changes / commits that you'd like in a separate PR?

[xezon]

Are there particular changes / commits that you'd like in a separate PR?

I would say any fixes that span multiple files are good candidates for separation.

[Caball009]

I'm inclined to leave out the TheSkirmishGameInfo and TheChallengeGameInfo leaks as I can't seem to find a good way to fix those leaks.

Is this PR going to be squashed and merged? Otherwise I'll have to rename the commit names.

[xezon]

In hindsight it would have been good if each fix had its own pull, because we needed to discuss quite a bit here.

We can merge with Squash or Rebase. Both are ok.

[xezon]

Is this ok to call always? When a replay auto ends, then there will be no quit menu to destroy?

[xezon]

This branch still contains the audio request fix. It needs to be removed.

[xezon]

Maybe delete them on game shutdown instead? Because it looks like they don't need to be recreated when all callsites already check for nullptr before allocation.