fix: support extended GeneralsOnline version format with build suffixes

[undead2146]

Summary

• Root cause: The Generals Online CDN changed the version format from MMDDYY_QFE# (2 segments) to MMDDYY_QFE#_SUFFIX (3+ segments, e.g. "042826_QFE3_EAC"). ParseGeneralsOnlineVersion required exactly 2 underscore-delimited parts, so it returned null for all new versions — update detection, version comparison, and sortable version IDs all silently broke.
• Fix: Changed the parser to accept 2+ segments, treating extra segments as build metadata that don't affect version ordering. Added a dedicated CompareGeneralsOnlineVersions method in VersionComparer that uses the structured parser instead of generic numeric comparison, ensuring "042826_QFE2" and "042826_QFE2_EAC" compare as equal.
• Impact: All 53 tests pass (existing + new). No changes to public API signatures — fully backward compatible with legacy 2-segment versions.

Changes

File	Change
GameVersionHelper.cs	parts.Length != 2 → parts.Length < 2 — accept extended format
VersionComparer.cs	New CompareGeneralsOnlineVersions() — structured date+QFE comparison with fallback
GeneralsOnlineUpdateService.cs	Updated comment to reflect extended format
GeneralsOnlineApiResponse.cs	Updated doc comment for version format
GeneralsOnlineRelease.cs	Updated doc comment for version format
GeneralsOnlineConstants.cs	Added remark clarifying separator usage

Tests added

Test file	Coverage
GameVersionHelperTests.cs (new)	Legacy format, extended format, multiple suffixes, null/empty, invalid date, non-numeric QFE, sortable version equality across formats
VersionComparerTests.cs	Extended format theory: same version, higher QFE, legacy vs extended equality, newer date
GeneralsOnlineJsonCatalogParserTests.cs	manifest.json with extended version, latest.txt fallback with extended version

Test plan

• All 53 version/comparison tests pass (20 new + 33 existing)
• Build succeeds with 0 warnings, 0 errors
• Backward compatible: legacy MMDDYY_QFE# versions still parse and compare correctly
• Forward compatible: extended MMDDYY_QFE#_SUFFIX versions parse date+QFE correctly
• Cross-format: legacy vs extended versions with same date+QFE compare as equal

🤖 Generated with Claude Code

---

🤖 OpenClaw Changes

Files changed: 2
Lines added: 7
Lines removed: 1
Branch: fix/go-version-extended-format
Commit: c88bdda0

Last updated: 2026-05-04T09:14:14.969Z

Greptile Summary

This PR fixes a version parsing regression caused by the Generals Online CDN switching from a 2-segment format (MMDDYY_QFE#) to a 3+-segment format (MMDDYY_QFE#_SUFFIX). The single-character guard change (!= 2 → < 2) in ParseGeneralsOnlineVersion restores update detection, and the new CompareGeneralsOnlineVersions method in VersionComparer ensures structured date+QFE ordering is used instead of opaque numeric fallback.

• GameVersionHelper.cs: Relaxes the segment-count guard from exactly 2 to at least 2, and replaces the hardcoded \"QFE\" literal with GeneralsOnlineConstants.QfeMarkerPrefix, aligning with the project's constants convention.
• VersionComparer.cs: Introduces CompareGeneralsOnlineVersions with explicit handling for both-null (generic fallback), single-null (unparseable treated as older), and fully-parsed (date then QFE) cases — directly addressing the asymmetric fallback identified in a prior review.
• Tests: 20 new tests across three files cover legacy format, extended format, multi-suffix, null/empty, invalid inputs, mixed-parseable comparison, and catalog-parser integration.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-tested relaxation of a length guard that restores broken version detection for the new CDN format.

The one-character logic fix is narrow and its correctness is directly validated by 20 new tests covering legacy, extended, multi-suffix, and edge-case inputs. The new CompareGeneralsOnlineVersions method explicitly handles all null-combination states, closing the asymmetric-fallback gap flagged in a prior review. No public API signatures changed, backward compatibility with existing 2-segment versions is confirmed, and the previously-hardcoded QFE literal is now correctly referenced through the constants class.

No files require special attention.

Important Files Changed

Filename	Overview
GenHub/GenHub.Core/Helpers/GameVersionHelper.cs	Core fix: relaxes parts.Length != 2 to < 2, uses QfeMarkerPrefix constant instead of hardcoded QFE, and updates doc comments for extended format support
GenHub/GenHub.Core/Helpers/VersionComparer.cs	Adds CompareGeneralsOnlineVersions() using structured date+QFE parsing; correctly handles both-null (fallback to numeric), single-null (unparseable treated as older), and fully-parsed cases; braces-only style cleanup elsewhere
GenHub/GenHub.Tests/GenHub.Tests.Core/Helpers/GameVersionHelperTests.cs	New test file with comprehensive coverage of legacy, extended, multi-suffix, null/empty, invalid date, non-numeric QFE, and sortable version equality cases
GenHub/GenHub.Tests/GenHub.Tests.Core/Helpers/VersionComparerTests.cs	New theory-based tests for extended format comparison, mixed-parseable fallback, and both-unparseable fallback; all assertions use Math.Sign correctly
GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs	Comment-only changes: adds tracking context for filesWithHashes and corrects a stale comment that incorrectly described the game-client manifest as including map files

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Version string input\ne.g. 042826_QFE3_EAC"] --> B["Split on underscore\nRemoveEmptyEntries"]
    B --> C{parts.Length less than 2?}
    C -- Yes --> D["Return null"]
    C -- No --> E["datePart = parts 0\nqfePart = parts 1 minus QFE prefix"]
    E --> F{Valid 6-char date\nand numeric QFE?}
    F -- No --> D
    F -- Yes --> G["Return DateTime + Qfe\nExtra suffix parts ignored"]

    G --> H["CompareGeneralsOnlineVersions"]
    H --> I{Parsed results?}
    I -- Both null --> J["CompareNumericVersions fallback"]
    I -- One null --> K["Null treated as older"]
    I -- Both parsed --> L["Compare Date then QFE"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Version string input\ne.g. 042826_QFE3_EAC"] --> B["Split on underscore\nRemoveEmptyEntries"]
    B --> C{parts.Length less than 2?}
    C -- Yes --> D["Return null"]
    C -- No --> E["datePart = parts 0\nqfePart = parts 1 minus QFE prefix"]
    E --> F{Valid 6-char date\nand numeric QFE?}
    F -- No --> D
    F -- Yes --> G["Return DateTime + Qfe\nExtra suffix parts ignored"]

    G --> H["CompareGeneralsOnlineVersions"]
    H --> I{Parsed results?}
    I -- Both null --> J["CompareNumericVersions fallback"]
    I -- One null --> K["Null treated as older"]
    I -- Both parsed --> L["Compare Date then QFE"]

Loading

_{Reviews (7): Last reviewed commit: "OpenClaw: Address review feedback from @..." | Re-trigger Greptile}

[ElTioRata]

Updater works again. EAC startup is still needed to play online.

[undead2146]

Updater works again. EAC startup is still needed to play online.

Does the user have to run EAC before running GO?

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs (1)

458-466: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

isPlugin is destructured but never referenced in either manifest-construction loop.

In the MapPack loop it is a pure dead variable (replace with _). In the GameClient loop it is also never tested — plugin files already reach manifestFiles.Add via the existing "skip maps only" path, so the isPlugin flag has no behavioral effect on manifest output (only on the earlier debug log at line 445).

If plugin files genuinely need different treatment in the future (e.g., a distinct InstallTarget), the logic should be added here; otherwise use discards to make the intent explicit and suppress any compiler warnings.

♻️ Proposed refactor — use discards where `isPlugin` is unused

-            foreach (var (relativePath, fileInfo, hash, isMap, isPlugin) in filesWithHashes)
+            foreach (var (relativePath, fileInfo, hash, isMap, _) in filesWithHashes)
             {
                 if (!isMap)
                 {
                     continue;
                 }
                 manifestFiles.Add(CreateMapManifestFile(relativePath, fileInfo, hash));
             }

-            foreach (var (relativePath, fileInfo, hash, isMap, isPlugin) in filesWithHashes)
+            foreach (var (relativePath, fileInfo, hash, isMap, _) in filesWithHashes)
             {
                 // ... existing logic unchanged
             }

Also applies to: 477-507

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs`
around lines 458 - 466, The destructured variable isPlugin in the
filesWithHashes loops is never used; update the tuple deconstruction to discard
that element (use _) in both the MapPack loop that calls CreateMapManifestFile
and the GameClient loop that adds to manifestFiles so the intent is explicit and
compiler warnings are suppressed; if special handling for plugins is required
later, add the conditional logic around isPlugin where manifestFiles.Add or
CreateMapManifestFile is invoked.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs`:
- Around line 417-419: The plugins directory string is hardcoded in
GeneralsOnlineManifestFactory (pluginsDirectory) while mapsDirectory uses
GeneralsOnlineConstants.MapsSubdirectory; add a new constant PluginsSubdirectory
to GeneralsOnlineConstants and replace the literal "plugins" in
GeneralsOnlineManifestFactory with GeneralsOnlineConstants.PluginsSubdirectory
so both directories follow the same pattern and are maintained centrally.

---

Outside diff comments:
In
`@GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs`:
- Around line 458-466: The destructured variable isPlugin in the filesWithHashes
loops is never used; update the tuple deconstruction to discard that element
(use _) in both the MapPack loop that calls CreateMapManifestFile and the
GameClient loop that adds to manifestFiles so the intent is explicit and
compiler warnings are suppressed; if special handling for plugins is required
later, add the conditional logic around isPlugin where manifestFiles.Add or
CreateMapManifestFile is invoked.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b6103e60-bf2c-41a6-a4e5-7e1e4a033ca4

📥 Commits

Reviewing files that changed from the base of the PR and between 2953657 and afff57e.

📒 Files selected for processing (1)

• GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs

[bobtista]

I'd move the EAC plugin work to another PR, make sure there is a real behavioral change, add tests, and try it to make sure it actually works.
nit: inconsistent usage of braces with single line if () {} blocks
Update the PR description too
Otherwise, I think the version-format fix looks solid