feat(stats): Add JSON game stats export alongside replay files Store stats

[bill-rich]

Summary

Adds a new StatsExporter module that writes game match statistics as a JSON file alongside the last replay file. When a recording stops, the exporter collects end-of-match stats from ScoreKeeper (and AcademyStats in Zero Hour) and writes them to a .json file in the replay directory.

The exporter also collects time-series snapshots every 30 frames (~0.5-1 second) during gameplay, capturing per-player resource and unit statistics over time. Snapshots are cleared on game start and flushed to the JSON output on recording stop.

Integration points

• Recorder::stopRecording() calls ExportGameStatsJSON() after saving the replay
• GameLogic::update() calls StatsExporterCollectSnapshot() each frame
• GameLogic::startNewGame() calls StatsExporterClearSnapshots() to reset state

Both titles

The implementation is provided for both Zero Hour (GeneralsMD) and Generals, with Zero Hour including additional AcademyStats accessors. The implementations are kept as close as possible across both titles.

[greptile-apps]

Greptile Summary

This PR introduces a new StatsExporter module for both Generals and Zero Hour that writes end-of-match statistics as a .gamestats.json file alongside the last replay file, and collects per-frame time-series snapshots during gameplay. The feature is opt-in via a new -exportStats command-line flag and integrates cleanly into Recorder::stopRecording(), GameLogic::update(), and GameLogic::startNewGame().

Key findings:

• Null pointer dereference (Zero Hour): player->getAcademyStats() at GeneralsMD/Code/GameEngine/Source/Common/StatsExporter.cpp:574 can return nullptr, but is unconditionally dereferenced starting at line 683. This will crash whenever the academy stats object is absent.
• Widespread inline if-body violations: Both StatsExporter.cpp files contain dozens of single-line if statements (e.g. if (!first) fprintf(f, ",\n");, if (dot != nullptr) *dot = '\0';, all the if (s > 0) fputc(',', f); loop guards), violating the project convention requiring statement bodies on separate lines for debugger breakpoint placement.
• The public accessor changes to ScoreKeeper.h (typedef promotion + 6 read-only getters) and AcademyStats.h (18 read-only getters) are clean and backward-compatible. License headers, #pragma once, and game-title strings are all correct for both titles.

Confidence Score: 3/5

• This PR should not be merged as-is due to a confirmed null pointer dereference in the Zero Hour build path.
• The integration logic and architecture are sound, and Generals-only builds are unaffected, but the missing null guard for academy in the Zero Hour ExportGameStatsJSON is a definite runtime crash waiting to happen. The pervasive inline if-body style violations are significant but non-blocking. Addressing the null check is required before merging.
• GeneralsMD/Code/GameEngine/Source/Common/StatsExporter.cpp requires attention for the null pointer dereference on academy. Both StatsExporter.cpp files require attention for the widespread inline if-body style violations.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/Common/StatsExporter.cpp	New 690-line file implementing JSON stats export for Generals. Contains widespread inline if-body violations (rule 16b9b669) and the if (dot != nullptr) *dot = '\0'; one-liner at line 532.
GeneralsMD/Code/GameEngine/Source/Common/StatsExporter.cpp	New 714-line Zero Hour variant. In addition to the same inline if-body violations as the Generals version, academy (retrieved via player->getAcademyStats() at line 574) is used unconditionally from line 683 onward with no null check, which will crash if the pointer is null.
GeneralsMD/Code/GameEngine/Include/Common/AcademyStats.h	Adds 18 public read accessors to AcademyStats. As noted in a previous review thread, getDragSelectUnits() and getUnitsEnteredTunnelNetwork() accessors are absent from this PR despite being listed in that earlier discussion. The 18 added here are all consumed in the academy JSON block.
Generals/Code/GameEngine/Include/Common/ScoreKeeper.h	Moves ObjectCountMap typedef from private to public scope and adds six read-only accessors needed by StatsExporter. Change is clean and backward-compatible.
GeneralsMD/Code/GameEngine/Include/Common/ScoreKeeper.h	Same typedef promotion and accessor additions as the Generals version. Clean and backward-compatible.
Generals/Code/GameEngine/Source/Common/Recorder.cpp	Adds a call to ExportGameStatsJSON in stopRecording() guarded by m_exportStats and a non-empty filename check. Integration is correct.
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	Zero Hour counterpart. Same clean integration as Generals Recorder.cpp.
Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Adds StatsExporterClearSnapshots() on new game start and StatsExporterCollectSnapshot() in the update loop, both gated behind m_exportStats. Integration is correct.
GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	Zero Hour counterpart. Same clean integration as Generals GameLogic.cpp.

Sequence Diagram

sequenceDiagram
    participant GL as GameLogic
    participant SE as StatsExporter
    participant REC as Recorder
    participant FS as FileSystem

    GL->>SE: startNewGame() → StatsExporterClearSnapshots()
    Note over SE: Resets snapshots vector,<br/>frame counter & player mapping

    loop Every logic frame (update)
        GL->>SE: StatsExporterCollectSnapshot()
        Note over SE: Skipped if < 30 frames since last snapshot
        SE->>SE: initPlayerMapping() (once)
        SE->>SE: Collect per-player stats snapshot
        SE->>SE: s_snapshots.push_back(snap)
    end

    GL->>REC: TheRecorder->UPDATE()
    REC->>REC: stopRecording() — saves .rep file

    alt m_exportStats && !m_fileName.isEmpty()
        REC->>SE: ExportGameStatsJSON(replayDir, replayFileName)
        SE->>SE: initPlayerMapping()
        SE->>FS: fopen("*.gamestats.json", "w")
        SE->>FS: Write game metadata block
        SE->>FS: Write players[] array (ScoreKeeper + AcademyStats)
        SE->>FS: writeTimeSeries() — columnar per-player arrays
        SE->>FS: fclose()
        SE->>SE: StatsExporterClearSnapshots()
    end

Loading

_{Last reviewed commit: 4b696c0}

[Skyaero42]

It looks like this is one of your first PR's for this repo. Thank you for your effort and contribution.

However, it would have been better to start an issue/discussion first about the need and requirements for such function, before implementing it.

Initial feedback:

• This should be an opt-in feature as it will impact performance during the game
• The JSON exporter is really hard to read. It would have been preferred to use an existing library, although I understand that would not work with VC6. Question is whether this feature is needed right now.

[bill-rich]

It looks like this is one of your first PR's for this repo. Thank you for your effort and contribution.

However, it would have been better to start an issue/discussion first about the need and requirements for such function, before implementing it.

Initial feedback:

• This should be an opt-in feature as it will impact performance during the game
• The JSON exporter is really hard to read. It would have been preferred to use an existing library, although I understand that would not work with VC6. Question is whether this feature is needed right now.

Thanks for taking a look!

Happy to add a flag so it is opt-in. Also, yeah. Not happy about the exporter, but got stuck because of VC6. I'm down for switching it out for something else if preferred.

I did start a discussion a while back(#1812), but it didn't get much traction. I figured I'd implement the feature and see what the feedback was in the PR.

The reason I'm trying to get this added is that there are some pretty cool ways to display the data once you've got it. I'm currently doing it by reading memory addresses and running replays, but having it built in would be way cleaner. Here is an example of what can be done with it: https://www.radarvan.com/

[github-actions]

⚠️ Title/Commit Validation Failed

Invalid commit messages:

• Remove unused public accessors
• Fix username
• Put stats behind flag
  PR titles and commit messages must follow conventional commits format:

type: Description
type(scope): Description

Allowed types: bugfix, build, chore, ci, docs, fix, feat, perf, refactor, revert, style, test, tweak, unify

See CONTRIBUTING.md for details.

[greptile-apps]

Null pointer dereference if getAcademyStats() returns null

academy is fetched at line 574 but there is no null check before it is first dereferenced at line 683 (academy->getSupplyCentersBuilt()). If player->getAcademyStats() ever returns nullptr (e.g. for an AI or for a player whose academy stats object was not initialised), this will crash at runtime.

The fix is to guard the entire "academy" block behind a null check:

if (academy != nullptr)
{
    fprintf(f, "      \"academy\": {\n");
    fprintf(f, "        \"supplyCentersBuilt\": %u,\n", academy->getSupplyCentersBuilt());
    // … rest of academy fields …
    fprintf(f, "      }\n");
}

Alternatively, simply assert/return early after retrieving academy if it is expected to always be non-null in this context.

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/Common/StatsExporter.cpp
Line: 574-683

Comment:
**Null pointer dereference if `getAcademyStats()` returns null**

`academy` is fetched at line 574 but there is no null check before it is first dereferenced at line 683 (`academy->getSupplyCentersBuilt()`). If `player->getAcademyStats()` ever returns `nullptr` (e.g. for an AI or for a player whose academy stats object was not initialised), this will crash at runtime.

The fix is to guard the entire `"academy"` block behind a null check:

```cpp
if (academy != nullptr)
{
    fprintf(f, "      \"academy\": {\n");
    fprintf(f, "        \"supplyCentersBuilt\": %u,\n", academy->getSupplyCentersBuilt());
    // … rest of academy fields …
    fprintf(f, "      }\n");
}
```

Alternatively, simply assert/return early after retrieving `academy` if it is expected to always be non-null in this context.

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