Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions GenHub/GenHub.Core/Constants/GameClientConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ public static class GameClientConstants
/// <summary>Standard retail Zero Hour directory name.</summary>
public const string ZeroHourRetailDirectoryName = "Command & Conquer Generals Zero Hour";

/// <summary>Parent directory of the native engine's default deploy tree, under the user's home.</summary>
public const string NativeDeployParentDirectoryName = "TheSuperHackers";

/// <summary>Directory name of the native engine's default Zero Hour deploy tree.</summary>
public const string NativeDeployZeroHourDirectoryName = "GeneralsZH";

// ===== GeneralsOnline Client Detection =====

/// <summary>GeneralsOnline 60Hz client executable name.</summary>
Expand Down
45 changes: 45 additions & 0 deletions GenHub/GenHub.Core/Constants/RetailArchiveConstants.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;

namespace GenHub.Core.Constants;
Expand Down Expand Up @@ -33,6 +35,19 @@ public static class RetailArchiveConstants
/// </remarks>
public const string ArchiveSearchPattern = "*.big";

/// <summary>
/// Filename suffix that marks an archive as Zero Hour content.
/// </summary>
/// <remarks>
/// A retail fact, not an engine one: a retail Zero Hour installation ships its
/// archives with this suffix (<c>INIZH.big</c>, <c>AudioZH.big</c>, …), verifiable
/// against a real installation, so any such archive marks Zero Hour data. Deliberately
/// not derived from any engine build's loading code, so the rule stays valid for a
/// stock retail install with no community client. Compare case-insensitively — retail
/// data copied from a disc or a Windows machine is frequently upper-cased.
/// </remarks>
public const string ZeroHourArchiveSuffix = "zh.big";

/// <summary>
/// How <see cref="ArchiveSearchPattern"/> is matched within a retail root.
/// </summary>
Expand Down Expand Up @@ -63,4 +78,34 @@ public static class RetailArchiveConstants
ZeroHourInstallPathVariable,
GeneralsInstallPathVariable,
];

/// <summary>
/// The canonical archive filenames of a retail Generals installation.
/// </summary>
/// <remarks>
/// A retail fact: these are the archives present in a retail Generals installation,
/// verifiable against a real one. Deliberately not derived from any engine build's
/// loading code, so the set stays valid for a stock retail install with no community
/// client. Any one of them marks a directory as holding Generals data — localised SKUs
/// vary in which language archives they carry, so requiring the full set would reject
/// valid installs. The comparer is case-insensitive for the same reason as
/// <see cref="ZeroHourArchiveSuffix"/>.
/// </remarks>
public static readonly IReadOnlySet<string> GeneralsArchiveNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
{
"audio.big",
"audioenglish.big",
"english.big",
"gensec.big",
"ini.big",
"maps.big",
"music.big",
"shaders.big",
"speech.big",
"speechenglish.big",
"terrain.big",
"textures.big",
"w3d.big",
"window.big",
};
}
76 changes: 76 additions & 0 deletions GenHub/GenHub.Core/Helpers/RetailArchiveClassifier.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
using System;
using System.IO;
using GenHub.Core.Constants;
using GenHub.Core.Models.GameInstallations;

namespace GenHub.Core.Helpers;

/// <summary>
/// Classifies a directory by the retail game archives it holds.
/// </summary>
/// <remarks>
/// This is the detection-side predicate on top of the <see cref="RetailArchiveConstants"/>
/// vocabulary: it decides <em>which</em> games' retail data a directory carries. It is
/// deliberately separate from the launch-side any-archive check
/// (<c>GameLauncher.ValidateRetailArchiveRoots</c>), which validates a root that has
/// already been chosen and stays game-agnostic so it cannot reject a valid root over a
/// localisation difference. Both build on <see cref="RetailArchiveConstants.ArchiveSearch"/>
/// so every call site matches archive files identically, case-insensitivity included.
/// </remarks>
public static class RetailArchiveClassifier
{
/// <summary>
/// Determines which games' retail archives are present in <paramref name="directory"/>.
/// </summary>
/// <param name="directory">The directory to classify.</param>
/// <returns>
/// The classification; an absent or null directory classifies as holding neither game.
/// A directory holding only unrecognised archives (mods, hotkey packs, control bars)
/// also classifies as neither — an arbitrary <c>.big</c> proves nothing about retail
/// data, which is exactly why the executable-name proxy this replaces was retired.
/// </returns>
/// <remarks>
/// Only the directory root is examined, never subdirectories: <c>Data/INI/INIZH.big</c>
/// is a duplicate shipped in the English, Chinese and Korean SKUs and must not be
/// counted twice. Filesystem errors (an unreadable directory, an I/O failure) propagate
/// to the caller rather than reading as "no archives" — converting a permission problem
/// into missing content is the failure mode
/// <see cref="RetailArchiveConstants.ArchiveSearch"/> exists to prevent.
/// </remarks>
public static RetailArchiveClassification ClassifyArchives(string? directory)
{
if (string.IsNullOrWhiteSpace(directory) || !Directory.Exists(directory))
{
return default;
}

var hasGenerals = false;
var hasZeroHour = false;

foreach (var archivePath in Directory.EnumerateFiles(
directory,
RetailArchiveConstants.ArchiveSearchPattern,
RetailArchiveConstants.ArchiveSearch))
{
var archiveName = Path.GetFileName(archivePath);

if (!hasZeroHour &&
archiveName.EndsWith(RetailArchiveConstants.ZeroHourArchiveSuffix, StringComparison.OrdinalIgnoreCase))
{
hasZeroHour = true;
Comment on lines +57 to +60

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Non-retail archives trigger installation detection

When a broad macOS search root such as Downloads contains a community archive like 340_ControlBarProZH.big, this suffix check marks the entire directory as a Zero Hour retail installation, causing unrelated files to be cached and processed as game content.

Knowledge Base Used: Game Detection: Installations and Client Identification

Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Helpers/RetailArchiveClassifier.cs
Line: 57-60

Comment:
**Non-retail archives trigger installation detection**

When a broad macOS search root such as Downloads contains a community archive like `340_ControlBarProZH.big`, this suffix check marks the entire directory as a Zero Hour retail installation, causing unrelated files to be cached and processed as game content.

**Knowledge Base Used:** [Game Detection: Installations and Client Identification](https://app.greptile.com/genhub/-/custom-context/knowledge-base/community-outpost/genhub/-/docs/game-detection.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}

if (!hasGenerals && RetailArchiveConstants.GeneralsArchiveNames.Contains(archiveName))
{
hasGenerals = true;
}

if (hasGenerals && hasZeroHour)
{
break;
}
}

return new RetailArchiveClassification(hasGenerals, hasZeroHour);
}
}
16 changes: 16 additions & 0 deletions GenHub/GenHub.Core/Interfaces/Manifest/IManifestProvider.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using GenHub.Core.Models.Enums;
using GenHub.Core.Models.GameClients;
using GenHub.Core.Models.GameInstallations;
using GenHub.Core.Models.Manifest;
Expand All @@ -23,5 +24,20 @@ public interface IManifestProvider
/// <param name="gameInstallation">The game installation for which to retrieve the manifest.</param>
/// <param name="cancellationToken">A cancellation token.</param>
/// <returns>The ContentManifest, or null if not found.</returns>
/// <remarks>
/// Surfaces one manifest only, preferring Zero Hour when both games are flagged.
/// For a combined installation carrying both games, use
/// <see cref="GetManifestAsync(GameInstallation, GameType, CancellationToken)"/>
/// per game so Generals is not skipped.
/// </remarks>
Task<ContentManifest?> GetManifestAsync(GameInstallation gameInstallation, CancellationToken cancellationToken = default);

/// <summary>
/// Asynchronously retrieves the manifest for one game of a game installation.
/// </summary>
/// <param name="gameInstallation">The game installation for which to retrieve the manifest.</param>
/// <param name="gameType">The game whose manifest is requested.</param>
/// <param name="cancellationToken">A cancellation token.</param>
/// <returns>The ContentManifest, or null if not found.</returns>
Task<ContentManifest?> GetManifestAsync(GameInstallation gameInstallation, GameType gameType, CancellationToken cancellationToken = default);
}
147 changes: 63 additions & 84 deletions GenHub/GenHub.Core/Models/GameInstallations/GameInstallation.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using GenHub.Core.Constants;
using GenHub.Core.Extensions.GameInstallations;
using GenHub.Core.Helpers;
using GenHub.Core.Interfaces.GameInstallations;
using GenHub.Core.Models.Enums;
using GenHub.Core.Models.GameClients;
Expand Down Expand Up @@ -105,17 +105,25 @@ public GameInstallation(
/// </summary>
/// <param name="generalsPath">The path to Generals, or null if not present.</param>
/// <param name="zeroHourPath">The path to Zero Hour, or null if not present.</param>
/// <remarks>
/// Each game's flag turns on when that game's retail archives are present in its
/// directory, not when an executable with a known name is. The executable name was
/// only ever a proxy for "the archives are here" — it rejects the canonical native
/// deploy, whose binary is extensionless — while the archives are the direct signal
/// and the thing a retail root actually has to supply. A combined directory carrying
/// both games' archives may legitimately be passed as both paths and sets both flags.
/// </remarks>
public void SetPaths(string? generalsPath, string? zeroHourPath)
{
if (!string.IsNullOrEmpty(generalsPath))
{
HasGenerals = Directory.Exists(generalsPath) && HasValidExecutable(generalsPath);
HasGenerals = ClassifyArchivesSafely(generalsPath).HasGeneralsArchives;
GeneralsPath = generalsPath;
}

if (!string.IsNullOrEmpty(zeroHourPath))
{
HasZeroHour = Directory.Exists(zeroHourPath) && HasValidExecutable(zeroHourPath);
HasZeroHour = ClassifyArchivesSafely(zeroHourPath).HasZeroHourArchives;
ZeroHourPath = zeroHourPath;
}

Expand All @@ -139,9 +147,9 @@ public void PopulateGameClients(IEnumerable<GameClient> clients)
}

/// <summary>
/// Initializes the installation by scanning for game directories and executables.
/// This method performs automatic detection of Generals and Zero Hour installations
/// within the installation path using standard directory naming conventions.
/// Initializes the installation by scanning for each game's retail archives.
/// Standard subdirectories are checked first, then the installation root itself,
/// which covers flat manual installs and combined directories holding both games.
/// </summary>
/// <remarks>
/// This method is primarily used for testing and initialization purposes.
Expand All @@ -158,97 +166,51 @@ public void Fetch()
bool foundZeroHour = false;

// 1. Check strict subdirectories first (standard structure)
var generalsPath = Path.Combine(InstallationPath, "Command and Conquer Generals");
if (Directory.Exists(generalsPath))
var generalsPath = Path.Combine(InstallationPath, GameClientConstants.GeneralsDirectoryName);
if (RetailArchiveClassifier.ClassifyArchives(generalsPath).HasGeneralsArchives)
{
var generalsExe = Path.Combine(generalsPath, GameClientConstants.GeneralsExecutable);
if (generalsExe.FileExistsCaseInsensitive())
{
HasGenerals = true;
GeneralsPath = generalsPath;
foundGenerals = true;
_logger?.LogDebug("Found Generals installation at {GeneralsPath}", GeneralsPath);
}
HasGenerals = true;
GeneralsPath = generalsPath;
foundGenerals = true;
_logger?.LogDebug("Found Generals installation at {GeneralsPath}", GeneralsPath);
}

var zeroHourPath = Path.Combine(InstallationPath, GameClientConstants.ZeroHourDirectoryName);
if (Directory.Exists(zeroHourPath))
if (RetailArchiveClassifier.ClassifyArchives(zeroHourPath).HasZeroHourArchives)
{
var zeroHourExe = Path.Combine(zeroHourPath, GameClientConstants.ZeroHourExecutable);
if (zeroHourExe.FileExistsCaseInsensitive())
{
HasZeroHour = true;
ZeroHourPath = zeroHourPath;
foundZeroHour = true;
_logger?.LogDebug("Found Zero Hour installation at {ZeroHourPath}", ZeroHourPath);
}
HasZeroHour = true;
ZeroHourPath = zeroHourPath;
foundZeroHour = true;
_logger?.LogDebug("Found Zero Hour installation at {ZeroHourPath}", ZeroHourPath);
}

// 2. If not found in subdirectories, check the root path (common for manual installs/repacks)
if (!foundGenerals)
// 2. If not found in subdirectories, check the root path itself. Archive
// classification tells the games apart even in one flat directory, so a
// combined root legitimately sets both flags to the same path — the earlier
// executable-based scan had to guess here, because both games ship the same
// executable name.
var rootClassification = RetailArchiveClassifier.ClassifyArchives(InstallationPath);

if (!foundGenerals && rootClassification.HasGeneralsArchives)
{
var rootGeneralsExe = Path.Combine(InstallationPath, GameClientConstants.GeneralsExecutable);

// Note: Zero Hour also has a generals.exe, so we need to be careful.
// If checking for valid installation, presence of generals.exe usually implies Generals capability.
if (rootGeneralsExe.FileExistsCaseInsensitive())
{
HasGenerals = true;
GeneralsPath = InstallationPath;
foundGenerals = true;
_logger?.LogDebug("Found Generals installation at root {GeneralsPath}", GeneralsPath);
}
HasGenerals = true;
GeneralsPath = InstallationPath;
foundGenerals = true;
_logger?.LogDebug("Found Generals installation at root {GeneralsPath}", GeneralsPath);
}

if (!foundZeroHour && string.IsNullOrEmpty(ZeroHourPath))
if (!foundZeroHour && rootClassification.HasZeroHourArchives)
{
// Zero Hour usually has generals.exe AND specific files like "generals.zh.exe" (sometimes) or just "generals.exe" with different hash/version.
// Detection primarily relies on folder name or presence of expansion files.
// Checking for generals.exe in root can map to both if the user selected a merged directory.
var rootGeneralsExe = Path.Combine(InstallationPath, GameClientConstants.GeneralsExecutable);

if (rootGeneralsExe.FileExistsCaseInsensitive())
{
// Check if Generals is already set to this path to avoid duplicate detection
// This prevents setting both GeneralsPath and ZeroHourPath to the same directory
// when platform-specific detectors (Steam/EA/etc) have already identified Generals here
bool isGeneralsAlreadySetToRoot =
!string.IsNullOrEmpty(GeneralsPath) &&
Path.GetFullPath(GeneralsPath).Equals(
Path.GetFullPath(InstallationPath),
StringComparison.OrdinalIgnoreCase);

if (!isGeneralsAlreadySetToRoot)
{
// If we are in root and found generals.exe, it could be ZH.
// Check for something specific to ZH if possible, or just assume if user pointed here it might be combined.
// For safety, let's treat root install as potentially containing both if we can't distinguish.

// Ideally we check for a ZH specific file, but standard detection often just looks for exe.
// Let's assume if the user pointed us here and it has the exe, it's valid.
// Standard Retail ZH has "generals.exe" but also usually lives in its own folder.
// If user pointed to "C:\Games\ZH", it has generals.exe.
HasZeroHour = true;
ZeroHourPath = InstallationPath;
foundZeroHour = true;
_logger?.LogDebug("Found Zero Hour installation at root {ZeroHourPath}", ZeroHourPath);
}
else
{
_logger?.LogDebug(
"Skipping Zero Hour detection at root {InstallationPath} - Generals already detected here",
InstallationPath);
}
}
HasZeroHour = true;
ZeroHourPath = InstallationPath;
foundZeroHour = true;
_logger?.LogDebug("Found Zero Hour installation at root {ZeroHourPath}", ZeroHourPath);
}

// Logic improvement: If we found generals.exe in root, we might have set BOTH to true/root.
// This is acceptable for some "All in One" repacks or if the user manually merged them.

// Log warnings only if absolutely nothing found
if (!foundGenerals && !foundZeroHour)
{
_logger?.LogWarning("No game executables found in {InstallationPath} or standard subdirectories", InstallationPath);
_logger?.LogWarning("No retail game archives found in {InstallationPath} or standard subdirectories", InstallationPath);
}

_logger?.LogInformation(
Expand Down Expand Up @@ -283,9 +245,26 @@ public override int GetHashCode()
return Id?.GetHashCode() ?? 0;
}

private static bool HasValidExecutable(string path)
/// <summary>
/// Classifies a directory's archives without letting a filesystem error escape.
/// </summary>
/// <param name="path">The directory to classify.</param>
/// <returns>The classification, or neither game when the directory cannot be read.</returns>
/// <remarks>
/// <see cref="SetPaths"/> is called from every platform detector, so it must not
/// throw. An unreadable directory is logged rather than silently reading as "no
/// archives" — the flag still ends up false, but the log names the real cause.
/// </remarks>
private RetailArchiveClassification ClassifyArchivesSafely(string path)
{
var possibleExes = new[] { GameClientConstants.SteamGameDatExecutable, GameClientConstants.GeneralsExecutable, GameClientConstants.ZeroHourExecutable };
return possibleExes.Any(exe => Path.Combine(path, exe).FileExistsCaseInsensitive());
try
{
return RetailArchiveClassifier.ClassifyArchives(path);
}
catch (Exception ex) when (ex is UnauthorizedAccessException or IOException)
{
_logger?.LogWarning(ex, "Could not read {Path} while classifying retail archives; treating it as holding none", path);
return default;
}
}
Comment on lines +248 to 269

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated ClassifyArchivesSafely in two files. Both files define the same private helper with the same UnauthorizedAccessException or IOException filter, because RetailArchiveClassifier exposes no safe entry point. Add one, for example RetailArchiveClassifier.TryClassifyArchives(string path, out RetailArchiveClassification classification), and let each caller log its own warning.

  • GenHub/GenHub.Core/Models/GameInstallations/GameInstallation.cs#L248-L269: replace the private helper with the shared classifier method and keep the existing warning log in SetPaths.
  • GenHub/GenHub.Windows/GameInstallations/WindowsInstallationDetector.cs#L314-L331: delete the private helper and call the shared classifier method from DetectRetailInstallations.
📍 Affects 2 files
  • GenHub/GenHub.Core/Models/GameInstallations/GameInstallation.cs#L248-L269 (this comment)
  • GenHub/GenHub.Windows/GameInstallations/WindowsInstallationDetector.cs#L314-L331
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@GenHub/GenHub.Core/Models/GameInstallations/GameInstallation.cs` around lines
248 - 269, The archive classification safety logic is duplicated across two
callers. In
GenHub/GenHub.Core/Models/GameInstallations/GameInstallation.cs#L248-L269,
replace ClassifyArchivesSafely with a shared
RetailArchiveClassifier.TryClassifyArchives(string path, out
RetailArchiveClassification classification) call while preserving SetPaths’s
existing warning log; in
GenHub/GenHub.Windows/GameInstallations/WindowsInstallationDetector.cs#L314-L331,
remove the duplicate helper and call the shared method from
DetectRetailInstallations, allowing each caller to retain its own warning
behavior.

}
Loading
Loading