bugfix(replay): Handle replay deletion during version mismatch prompt

[bobtista]

Fixes #1732

Summary

• Revalidates the replay after the version-mismatch prompt.
• Keeps the Replay Menu open, shows “REPLAY NOT FOUND,” and refreshes the list if validation or final playback fails.
• Enters recorder playback mode only after initialization succeeds, preventing a failed open from leaving a null replay file.
• Applies the fix to both Generals and Zero Hour.

Notes

• Both reallyLoadReplay() and loadReplay() now handle playbackFile() failure by keeping the menu open, showing the replay-not-found message, and refreshing the list.
• Recorder playback mode is only entered after initialization succeeds.
• The changes are mirrored across Generals and Zero Hour.
• Side benefit now ReplayMenu.cpp can be unified to core

Testing

1. Select a replay with a different executable hash/version.
2. Double-click it or press Load.
3. Delete the replay externally while the version-mismatch prompt is open.
4. Confirm the prompt.
5. Verify that the game does not crash, displays “REPLAY CANNOT BE LOADED,” and refreshes the replay list.

[Stubbjax]

There is already an established pathway for handling file deletion during a confirmation popup in ReplayMenu::delateReplay. Adopting this logic would simplify the implementation and resolve the message and list refresh inconsistencies of this change, which I've tabulated below for clarity.

Action	Refreshes list	Message
DELETE REPLAY	Yes	The system cannot find the specified file.
LOAD REPLAY	No	This replay cannot be loaded because the file no longer exists on this device.

[bobtista]

There is already an established pathway for handling file deletion during a confirmation popup in ReplayMenu::delateReplay. Adopting this logic would simplify the implementation and resolve the message and list refresh inconsistencies of this change, which I've tabulated below for clarity.

Action Refreshes list Message
DELETE REPLAY Yes The system cannot find the specified file.
LOAD REPLAY No This replay cannot be loaded because the file no longer exists on this device.

Thank you - I implemented it this way, rebased the latest main, and force pushed

[greptile-apps]

Greptile Summary

This PR hardens replay loading in both game variants.

• Revalidates replay metadata and map availability after the version-mismatch prompt.
• Keeps the replay menu visible and refreshes its list when final playback initialization fails.
• Defers pending-map and playback-mode state until the first replay frame is read successfully.

Confidence Score: 3/5

The PR is not yet safe to merge because restarting a damaged or deleted replay can still leave both recorder implementations in playback mode with a null replay file.

The deferred mode assignment fixes replay loading from the menu when the recorder begins outside playback mode, but replay restart calls playbackFile while playback mode is already active; the failed initial-frame path closes the file and returns without clearing that pre-existing mode.

Files Needing Attention: Generals/Code/GameEngine/Source/Common/Recorder.cpp; GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/Common/Recorder.cpp	Defers playback state until initialization succeeds, but an existing playback mode survives the new early-return path during replay restart.
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	Mirrors the recorder initialization change and retains the same replay-restart failure.
Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp	Adds generic load-failure handling, post-prompt revalidation, a repeated map guard, and conditional menu hiding.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp	Mirrors the corrected replay-menu validation and failure behavior for Zero Hour.

_{Reviews (11): Last reviewed commit: "bugfix(replay): Reject playback after fa..." | Re-trigger Greptile}

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[bobtista]

@greptileai

[bobtista]

The latest push does this:

• Use readReplayMapInfo() to validate the file before playback (same pattern as loadReplay)
• Show the "REPLAY NOT FOUND" message that loadReplay uses
• Refresh the listbox on failure (matching deleteReplay behavior)

[bobtista]

There is already an established pathway for handling file deletion during a confirmation popup in ReplayMenu::delateReplay. Adopting this logic would simplify the implementation and resolve the message and list refresh inconsistencies of this change, which I've tabulated below for clarity.

Action Refreshes list Message
DELETE REPLAY Yes The system cannot find the specified file.
LOAD REPLAY No This replay cannot be loaded because the file no longer exists on this device.

The latest push uses readReplayMapInfo() to validate the file before playback (like loadReplay), shows the "REPLAY NOT FOUND" message, and refreshes the listbox on failure. Both the message and list refresh should now be consistent with deleteReplay. Ready for re-review when you get a chance.

[Stubbjax]

The latest push uses readReplayMapInfo() to validate the file before playback (like loadReplay), shows the "REPLAY NOT FOUND" message, and refreshes the listbox on failure. Both the message and list refresh should now be consistent with deleteReplay. Ready for re-review when you get a chance.

Is the latest push actually pushed? Issues still persist:

1. The NOT FOUND message is inconsistent between load and delete actions.
2. When attempting to load a replay, the game crashes after clicking OK on the NOT FOUND message.

[bobtista]

The latest push uses readReplayMapInfo() to validate the file before playback (like loadReplay), shows the "REPLAY NOT FOUND" message, and refreshes the listbox on failure. Both the message and list refresh should now be consistent with deleteReplay. Ready for re-review when you get a chance.

Is the latest push actually pushed? Issues still persist:

1. The NOT FOUND message is inconsistent between load and delete actions.
2. When attempting to load a replay, the game crashes after clicking OK on the NOT FOUND message.

Yes latest was pushed, but you're right. RE Inconsistent messages — I'll switch to using the same FormatMessage/GetLastError pattern that deleteReplay uses, with "GUI:Error" as the title. RE Crash after clicking OK, I think that's something else - I'll investigate.

[xezon]

Needs re-review @Stubbjax

[xezon]

@Stubbjax

[bobtista]

Reworked this.

RE message consistency, reallyLoadReplay now re-validates with readReplayMapInfo() and shows the same "REPLAY NOT FOUND" message as loadReplay, instead of the raw OS FormatMessage string.

RE Crash after OK, RecorderClass::playbackFile(): it was setting m_mode = RECORDERMODETYPE_PLAYBACK before opening the file, so a failed open left the recorder in playback mode with a NULL m_file, and the next update dereferenced it. Moved the assignment after the successful header read.

Also drops the Win32/VC6 file-attribute code :) Ready for re-review.

[bobtista]

Apologies - on 19 Jul I committed locally but never pushed, so you were both reviewing the February version. Pushed now and rebased onto current main.

[xezon]

The bot has a complaint.

[xezon]

Code looks ok

stale