Conversation
6933dd3 to
351db69
Compare
bobtista
added
Enhancement
351db69 to
c89d8fc
Compare
de4bd76 to
dfde359
Compare
dfde359 to
10d9370
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDebug builds add command-line options to save a game at a requested logic frame and set the output filename. Game logic handles the save request when the frame threshold is reached. GeneralsMD also returns an exit code based on the save outcome or unsupported game mode. ChangesCommand-line frame save
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant CommandLine
participant GlobalData
participant GameLogic
participant GameEngine
participant GameMain
CommandLine->>GlobalData: Store requested frame and filename
GameLogic->>GlobalData: Read save settings
GameLogic->>GameLogic: Check frame and UI input state
GameLogic->>GameLogic: Write configured save
GameLogic->>GameEngine: Request shutdown and set exit code
GameEngine->>GameMain: Return exit code after execution
Merge Risk: 🟡 Moderate · up to Unattended save tests can report success when no usable save was written. Propagate save failures to the process exit status before merging. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Saving is limited to debug builds and a filename in the save directory, but the two game editions handle unsupported modes and failed saves differently. In Generals, an automated run can report success when its save failed. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 75f0284a-2331-4f9b-911d-fede3ead5fa9
📒 Files selected for processing (10)
Core/GameEngine/Source/Common/CommandLine.cppGenerals/Code/GameEngine/Include/Common/GlobalData.hGenerals/Code/GameEngine/Source/Common/GlobalData.cppGenerals/Code/GameEngine/Source/GameLogic/System/GameLogic.cppGeneralsMD/Code/GameEngine/Include/Common/GameEngine.hGeneralsMD/Code/GameEngine/Include/Common/GlobalData.hGeneralsMD/Code/GameEngine/Source/Common/GameEngine.cppGeneralsMD/Code/GameEngine/Source/Common/GameMain.cppGeneralsMD/Code/GameEngine/Source/Common/GlobalData.cppGeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 8a315147-3dc0-4bfd-a8e2-1456dfdd15bb
📒 Files selected for processing (2)
Core/GameEngine/Source/Common/CommandLine.cppGenerals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Propagate failed saveGame results to the Generals process status. · GameLogic.cpp:3220-3232
Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp:3220-3232
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate failed
saveGameresults to the Generals process status.
-saveatframeis registered for unattended save/load tests. A failedsaveGamecall in debug Generals is logged, butGameMainstill returns its initial zero status. An unattended caller can therefore report success for a failed save. Add the same exit-status path already used by GeneralsMD.Suggested fix
diff --git a/Generals/Code/GameEngine/Include/Common/GameEngine.h b/Generals/Code/GameEngine/Include/Common/GameEngine.h @@ virtual void setQuitting( Bool quitting ); ///< set quitting status virtual Bool getQuitting(); ///< is app getting ready to quit. + Int getExitCode() const { return m_exitCode; } + void setExitCode(Int exitCode) { m_exitCode = exitCode; } + @@ Real m_logicTimeAccumulator; ///< Frame time accumulated towards submitting a new logic frame + Int m_exitCode; ///< process exit status after engine shutdown Bool m_quitting; ///< true when we need to quit the game Bool m_isActive; ///< app has OS focus. }; diff --git a/Generals/Code/GameEngine/Source/Common/GameEngine.cpp b/Generals/Code/GameEngine/Source/Common/GameEngine.cpp @@ // initialize to non garbage values m_logicTimeAccumulator = 0.0f; + m_exitCode = 0; m_quitting = FALSE; m_isActive = FALSE; diff --git a/Generals/Code/GameEngine/Source/Common/GameMain.cpp b/Generals/Code/GameEngine/Source/Common/GameMain.cpp @@ // run it TheGameEngine->execute(); + exitcode = TheGameEngine->getExitCode(); } diff --git a/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp b/Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp @@ - MAYBE_UNUSED const SaveResult saveResult = TheGameState->saveGame(saveName, UnicodeString(L"Command line save"), SAVE_FILE_TYPE_NORMAL); + const SaveResult saveResult = TheGameState->saveGame(saveName, UnicodeString(L"Command line save"), SAVE_FILE_TYPE_NORMAL); DEBUG_LOG(("Command line save to '%s' at frame %d returned %d", saveName.str(), m_frame, (Int)saveResult.saveCode)); TheWritableGlobalData->m_saveAtFrame = 0; + TheGameEngine->setExitCode(saveResult.saveCode == SC_OK ? 0 : 1); TheGameEngine->setQuitting(TRUE);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: ee51659e-65f9-4460-9b90-b4ae043c7d02
📒 Files selected for processing (1)
Core/GameEngine/Source/Common/CommandLine.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- Core/GameEngine/Source/Common/CommandLine.cpp
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
1 participant
Fixes #3134. Follow-up to #3001.
There is no command-line way to write a save, so save/load testing still needs the in-game menu.
Now
-saveatframe <frame>saves before logic updates at or after the requested frame, then quits. It waits while player input is disabled.-saveto <filename>sets the filename, defaulting tocommandline.sav. Both options are debug-only and reject missing or invalid arguments.Zero Hour also rejects multiplayer and replay saves, and returns a nonzero exit status if saving fails.
Earlier verification on
MD_CHI01: a save requested at frame 1780 deferred to 1781 while input was disabled, then loaded through-loadsave.The follow-up changes pass focused C++98 tests for argument validation, save deferral, game modes, save failures and exit-status propagation through cleanup. No new full game build or in-game save/load test was run.
Note
Add
-saveatframeand-savetodebug command-line options-saveatframe(positive frame number) and-saveto(filename,.savappended if missing) parsers in CommandLine.cpp and CommandLine.cpp; invalid values terminate startupRTS_DEBUG-onlyGlobalDatamembers, defaulting tocommandline.savand frame zero (disabled)GameLogic::updatewrites a normal save before script and object updates, then requests shutdown; the save is deferred while in-game input is disabledGameEnginegetter/setter andGameMainuses it after normal execution); in GeneralsMD, a failed save exits with 1, a successful save exits with 0, and multiplayer/replay sessions exit with 1 without savingRTS_DEBUG) buildsMacroscope summarized 968003c. (Automatic summaries will resume when PR exits draft mode or review begins).