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 parse failure introduced by the Generals Online CDN switching from MMDDYY_QFE# (2 segments) to MMDDYY_QFE#_SUFFIX (3+ segments, e.g. "042826_QFE3_EAC"). The one-line guard change in ParseGeneralsOnlineVersion (!= 2 → < 2) restores correct parsing, and the new CompareGeneralsOnlineVersions method in VersionComparer replaces the former generic numeric fallback with structured date+QFE comparison — including proper null-case handling for mixed parseable/unparseable inputs (the previously flagged asymmetric fallback issue is fully addressed here). Test coverage is thorough across all new and existing paths.

Confidence Score: 5/5

Safe to merge — all logic changes are correct, backward compatible, and well-tested.

No P0 or P1 issues found. The core parser change is minimal and correct. The new comparison method handles all null combinations properly. The isPlugin flag in the manifest factory is intentionally diagnostic-only and clearly documented. All 53 tests pass including 20 new cases covering the edge conditions.

No files require special attention.

Important Files Changed

Filename	Overview
GenHub/GenHub.Core/Helpers/GameVersionHelper.cs	Core fix: parts.Length != 2 changed to parts.Length < 2 in ParseGeneralsOnlineVersion; hardcoded "QFE" replaced with GeneralsOnlineConstants.QfeMarkerPrefix. Logic is correct and handles all edge cases via the surrounding try-catch.
GenHub/GenHub.Core/Helpers/VersionComparer.cs	New CompareGeneralsOnlineVersions private method correctly handles all null combinations (both null → numeric fallback, one null → treat as older, both parsed → date-then-QFE). Addresses the previously flagged asymmetric fallback.
GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineManifestFactory.cs	Adds isPlugin flag and PluginsSubdirectory detection for EAC files. The flag is used only in LogDebug; plugin files are routed to GameClient manifests with Workspace install target, consistent with all other non-map files. Intent is clearly documented in comments.
GenHub/GenHub.Tests/GenHub.Tests.Core/Helpers/GameVersionHelperTests.cs	New test file covering legacy format, extended format, multiple suffixes, null/empty/whitespace, no-underscore, invalid date, non-numeric QFE, sortable version encoding, and cross-format equality. Comprehensive coverage of all parser branches.
GenHub/GenHub.Tests/GenHub.Tests.Core/Helpers/VersionComparerTests.cs	Adds theory tests for extended-format comparison (same QFE, higher QFE, legacy-vs-extended, newer date), mixed parseable/unparseable cases, and both-unparseable fallback. Covers all key correctness scenarios including the previously problematic asymmetric case.
GenHub/GenHub.Core/Constants/GeneralsOnlineConstants.cs	Adds PluginsSubdirectory = "plugins" constant and a remark on QfeSeparator clarifying catalog-parser vs. comparison usage. Consistent with the constants style guide.
GenHub/GenHub/Features/Content/Services/GeneralsOnline/GeneralsOnlineUpdateService.cs	Comment-only change updating the format description from MMddyy_QFE# to MMddyy_QFE#[_SUFFIX]. No logic changes.
GenHub/GenHub.Core/Models/GeneralsOnline/GeneralsOnlineApiResponse.cs	Doc-comment update to reflect extended version format. No logic changes.
GenHub/GenHub.Core/Models/GeneralsOnline/GeneralsOnlineRelease.cs	Doc-comment update to reflect extended version format. No logic changes.
GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Content/Services/GeneralsOnline/GeneralsOnlineJsonCatalogParserTests.cs	Two new tests: extended-version manifest.json parsing and latest.txt fallback with extended format. Both verify the version string is preserved verbatim and download URL is correctly carried through.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CompareVersions called\nwith publisherType=GeneralsOnline] --> B[CompareGeneralsOnlineVersions]
    B --> C[ParseGeneralsOnlineVersion v1 and v2]
    C --> D{Both null?}
    D -- Yes --> E[CompareNumericVersions fallback]
    D -- No --> F{v1 null?}
    F -- Yes --> G[return -1: v1 treated as older]
    F -- No --> H{v2 null?}
    H -- Yes --> I[return 1: v2 treated as older]
    H -- No --> J[Compare Date fields]
    J --> K{Dates equal?}
    K -- No --> L[return dateCompare]
    K -- Yes --> M[Compare QFE integers]
    M --> N[return qfeCompare]

    subgraph ParseGeneralsOnlineVersion
        P1[Split on underscore RemoveEmptyEntries] --> P2{parts.Length < 2?}
        P2 -- Yes --> P3[return null]
        P2 -- No --> P4[datePart=parts0 qfePart=parts1 strip QFE]
        P4 --> P5{6-char date AND numeric QFE?}
        P5 -- No --> P3
        P5 -- Yes --> P6[Parse MM DD YY Return Date+QFE tuple]
        P6 --> P7[Extra suffix segments parts2+ ignored]
    end

Loading

_{Reviews (6): Last reviewed commit: "OpenClaw: Address code review feedback f..." | 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