chore: alpha 4 release - #229
Conversation
…trols - Added `NullableDecimalToIntConverter` to handle data conversion between View and ViewModel. - Defined a new "phantom" style for `NumericUpDown` to match the existing minimalist UI. - Replaced `TextBlock` value displays with `NumericUpDown` controls for Gamma, Audio volumes, Font sizes, and Camera settings. - Adjusted Grid column widths to accommodate interactive input fields.
Added manual folder picker for Retail GameInstallations without registry
…227) Fixed GameSettings not persisting whenever the profile was saved.
… resolution (#225) * fix(deps): ensure disabled game installation is auto-resolved when required by client * feat(local): add GameType selection and validation for local content * feat(game-profiles): hide GameInstallation from UI
…ate detection (#228) Upon launching a GameProfile that has a GeneralsOnline GameClient, a check for an update will be made. When an update is available it will download the new release, remove the old GameClient and MapPack, reconcile the profiles with the new release, then continue with launching the game.
Users can now subscribe to development branches to receive notifications for every new push or update
…xternal JSON (#205) This commit lays the foundation for the Provider Defition framework whereby publishers can become ContentProviders in GenHub.
… data` button under Danger Zone (#230)
… management (#235) - Added a GameType filter for Generals and Zero Hour. - Updated the selected ContentType button to highlight when active. - Centered all ContentItems. - Organized ContentItems into a brick-style layout. - Replaced the add/remove buttons with a click action on the ContentItem itself. - Set the "Delete" button to be visible only on hover.
Added a section under settings for the following data directories; workspace, profiles, manifests and CAS-pool,
…234) Removed old local content dialog properties and methods from GameProfileSettingsViewModel. Enhanced UI with drag-and-drop support for file and folder imports.
39d9455 to
b9f8255
Compare
…age and multi-instance debugging support (#239) * Added persistent storage for manual game installations and multi-instance debugging support. * Improved update reliability with HTTP retries, GitHub API caching, and enhanced error handling. * Refactored constants and implemented automated cleanup for orphaned workspace directories.
b9f8255 to
615dc70
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| try | ||
| { | ||
| manualInstallationStorage.ClearAllAsync(default).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
logic: Using GetAwaiter().GetResult() inside a lock can cause deadlocks. The _cacheLock is held synchronously while waiting for an async operation to complete.
| manualInstallationStorage.ClearAllAsync(default).GetAwaiter().GetResult(); | |
| await manualInstallationStorage.ClearAllAsync(cancellationToken).ConfigureAwait(false); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/GameInstallations/GameInstallationService.cs
Line: 227:227
Comment:
**logic:** Using `GetAwaiter().GetResult()` inside a lock can cause deadlocks. The `_cacheLock` is held synchronously while waiting for an async operation to complete.
```suggestion
await manualInstallationStorage.ClearAllAsync(cancellationToken).ConfigureAwait(false);
```
How can I resolve this? If you propose a fix, please make it concise.…port and GitHub rate limit tracking (#237) This update introduces a persistent notification feed with history tracking and a redesigned bell UI featuring dynamic unread badges. It adds support for multi-action notifications with styled buttons and integrates a GitHub rate limit tracker to provide real-time API usage warnings. The core notification architecture has been refactored into record types for better immutability, alongside service enhancements for managing read states and history. Additionally, the UI features a new custom title bar and redesigned toasts, backed by comprehensive unit tests to ensure thread safety and reliability.
…y launching (#241) * feat: Implement SteamLauncher for game directory preparation and proxy launching - Added SteamLauncher service to manage game directory preparation for Steam-tracked profiles. - Introduced ProxyConfig for configuring the proxy launcher with target executable and working directory. - Implemented methods for deploying the proxy launcher and creating necessary configuration files. - Enhanced manifest generation to support backup executable handling from the Steam Proxy Launcher. - Updated SteamManifestPatcher to improve executable validation logic. - Refactored HardLinkStrategy to prioritize file deduplication based on content type. - Improved error handling and logging throughout the game installation and manifest processes. - Updated documentation to reflect changes in detection and manifest creation workflows. * fix: resolve proxy launcher not replacing generals.exe * chore: resolve greptile comments * fix: resolve GameProfileSettingsWindow drag to resize from fullscreen
) * feat: add GitHub Pages deployment workflow and landing page assets * feat: Add Replay Manager tool with drag-and-drop functionality and file management - Implemented ReplayManagerView for managing replay files. - Added drag-and-drop support for importing replay files (.rep, .zip). - Enhanced ToolsViewModel to include Replay Manager services. - Updated UI styles for better visual consistency. - Introduced EnumToBoolConverter and EqualsToConverter for improved data binding. - Added documentation for Replay Manager features and usage. - Registered Replay Manager services in the dependency injection module. * feat(tools): enhance map/replay managers and refine tools sidebar ui * feat(tools): enhance map/replay managers and refine tools sidebar ui * feat(tools): enhance map/replay managers and refine tools sidebar ui * fix: resolve build version mismatch and the add local content DragDrop not working admin
|
Too many files changed for review. ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… for the website (#270)
…x state persistence and race conditions Implemented mute/unmute support in the notifications feed with proper UI indicators and command bindings. Converted mute operations to async with reliable persistence via UserSettingsService and added CancellationToken support. Fixed race conditions affecting mute state and ensured state updates only occur after successful persistence. Improved notification badge sizing, positioning, and count handling. Refactored bindings to rely on ViewModel state, removed dead code, and resolved StyleCop issues. Includes UI fixes, cleanup, and reliability improvements.
…th genpatcher registry. (#273) This PR fixes the Controlbar and Hotkeys not being converted and packed correctly into `.big` files.
…gement (#261) * GeneralsOnline, CommunityOutpost and TheSuperhackers will auto-update and reconcile GameProfiles that have outdated content replacing them with the updated content.
…ware (#328) * fix(settings): resolve Options.ini per platform instead of always using Windows paths * fix(config): honor the configured application data path in all runtime consumers * refactor(manifest): consolidate executable classification into one rule * feat(manifest): add platform variants and deterministic entry point resolution * feat(workspace): implement Unix hard links and replace shell-based permission checks * fix(workspace): make symlink strategies reachable on Unix * fix(workspace): set the execute bit on a workspace copy instead of the shared blob * fix(workspace): materialize executables atomically instead of delete-then-move Copy, set the mode on the copy, then rename over the destination. The previous copy/delete/move/chmod sequence exposed two states: destination missing, and destination present but not executable. The second is unrecoverable because verification never mutates, so the workspace stays broken for every later launch. Closes #316. * feat(manifest): reject variant manifests at ingestion until consumers are migrated Deliverers, validators, CAS reference counting and GC all still read ContentManifest.Files, which is empty for a variant manifest. Accepting one would deliver nothing while reporting success and record the wrong blobs in reference counting. Adds a fail-closed gate applied in ManifestDiscoveryService and ManifestProvider, and a manifest format version identifying variant manifests so the rejection is attributable rather than presenting as a parse failure. Temporary: remove as part of the resolved-variant migration (#321). Closes #314. * chore: clear build warnings introduced by this branch development builds warning-clean; these were introduced here. Covers using-directive ordering, comment spacing, and a platform guard the analyzer can follow for File.SetUnixFileMode inside the copy lambda. * chore: clear cross-platform style warnings * chore(workspace): order static helper before instance members * chore(manifest): extract the ingestion-gate check and guard the root-only test case The deserialize, gate, log-and-skip sequence was repeated at all three ManifestDiscoveryService ingestion points, differing only in the context label. Collapsed into IsManifestAccepted. The other-only execute-bit test now skips for uid 0. Root bypasses the permission bits, so faccessat reports execute access and the behaviour under test does not exist. geteuid is used rather than the user name, which is wrong under 'sudo -E' and for any uid-0 account named otherwise. GitHub's runners are non-root so this passed in CI, but it fails in a root container. * fix(launching): attribute the symlink downgrade to capability rather than admin rights The downgrade is driven by a real symlink-creation probe, so the messages no longer claim elevation is required. symlink(2) needs no privilege on Unix, and Windows grants it under Developer Mode, so telling the user to elevate sent them after a fix that would not help. The user-facing notification carried the same claim and is corrected too. Genuine Windows UAC handling for Error 740 is unchanged. Also covers the ingestion gate at the pool, which is the chokepoint every deliverer, resolver and detector reaches. The tests assert that a variant manifest is rejected before any content is stored and before any CAS reference is tracked; mis-tracked references are what corrupts reference counting and garbage collection, so returning a failure alone would not be enough. Both fail if the gate is removed.
* feat(macos): add local application host * feat(macos): disable self-update, add installation detector and composition-root tests * feat(macos): add CI lane, app bundle, AVIF platform guard and startup fix * fix(macos): report a denied privacy prompt instead of no installations found The detector swallowed UnauthorizedAccessException and returned null, which is how a declined TCC prompt for a protected location such as ~/Documents surfaces. Detection then reported success with zero installations, telling the user they own no games when we were simply not allowed to look, with no path to recovery. Denied roots are now tracked and named. When nothing was found and a location was denied, detection reports failure with the System Settings path to grant access, which also stops callers caching the empty result. Closes #319. * chore: clear build warnings introduced by this branch development builds warning-clean; these were introduced here. Moves the token-store explanation out of the argument list so it reads as a comment rather than a stray parameter, and drops trailing whitespace from the solution file. * chore: clear macOS host style warnings * fix(macos): isolate composition tests and retry partial scans * refactor(macos): resolve standard search paths by platform * test: serialize isolated application settings by type * ci: name macOS smoke-test duration * refactor(macos): streamline failed installation scans
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // Then discover from local filesystem locations. | ||
| // Routed through the configuration provider so a user-relocated data directory | ||
| // is honoured; a raw SpecialFolder lookup would keep reading the default tree. | ||
| var applicationDataPath = configurationProvider.GetApplicationDataPath(); |
There was a problem hiding this comment.
CRITICAL: Missing null check for GetApplicationDataPath() return value
Path.Combine will throw ArgumentNullException if configurationProvider.GetApplicationDataPath() returns null, causing runtime failure. Add null validation before using the result.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| await Task.Run( | ||
| () => | ||
| { | ||
| if (UnixNativeMethods.Link(absoluteTargetPath, absoluteLinkPath) == 0) |
There was a problem hiding this comment.
CRITICAL: No path validation before calling UnixNativeMethods.Link
The absoluteLinkPath and absoluteTargetPath are not validated to be within expected workspace/CAS bounds before creating hard links. This creates a security vulnerability allowing hard links to arbitrary files on the system.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // Gating the discovery and provider services alone left those paths open. | ||
| if (!ManifestIngestionGate.TryAccept(manifest, out var variantRejection)) | ||
| { | ||
| errors.Add(variantRejection!); |
There was a problem hiding this comment.
WARNING: Null-forgiving operator used without guarantee of non-null value
variantRejection! assumes rejectionReason is non-null when TryAccept returns false. While the implementation sets it, the signature allows null. Consider explicit null check or defensive programming.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| { | ||
| if (!ManifestIngestionGate.TryAccept(manifest, out var rejectionReason)) | ||
| { | ||
| throw new ManifestValidationException(requestedId, rejectionReason!); |
There was a problem hiding this comment.
WARNING: Null-forgiving operator used without guarantee of non-null value
rejectionReason! assumes rejectionReason is non-null when TryAccept returns false. While the implementation sets it, the signature allows null. Consider explicit null check or defensive programming.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| try | ||
| { | ||
| // Replace the entry with a private copy so the mode change cannot reach a |
There was a problem hiding this comment.
SUGGESTION: Predictable temporary file naming could enable race conditions
Using targetPath + ".genhub-exec-tmp" creates predictable temporary file names. Consider using Path.GetRandomFileName() or a GUID-based approach to reduce predictability and potential race conditions.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| internal static bool CanExecute(string path) | ||
| { | ||
| const int executeMode = 1; | ||
| var currentWorkingDirectory = OperatingSystem.IsMacOS() ? -2 : -100; |
There was a problem hiding this comment.
SUGGESTION: Magic numbers lack named constants or documentation
The values -2/-100 (AT_FDCWD) and 0x10/0x200 (AT_EACCESS) are used without named constants or explanatory comments, reducing code maintainability. Consider defining these as named constants with POSIX-standard names.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…331) #328 made IGamePathProvider a required dependency and #329 added the macOS host, but neither could carry the macOS registrations: they reference types from the first and a module from the second, so they only compile once both are in. Each PR was green in isolation and the merged state is not — GenHub.app now aborts at container build with 'Unable to resolve service for type IGamePathProvider while attempting to activate GameSettingsService', cascading across roughly twenty descriptors including MainViewModel and IGameLauncher. Registers IGamePathProvider, ISymlinkCapabilityProvider and IFileOperationsService for macOS. Only the first breaks startup; without the other two macOS silently copies instead of hard-linking and cannot reach symlink strategies, which are the behaviours #328 set out to fix. Also moves the macOS test run before publish. GenHub.Tests.MacOS was already covered by the job's final 'Run Tests' step, but that sits after Publish and Smoke Test App Launch, so the startup crash killed the job first and the assertion naming the service never ran. MacOSHost_ResolvesEveryRequiredService already asserted this; it was reached too late to be useful. The project is skipped in the later sweep so it runs once.
|
Superseded by #335 — closed after the fact, so here is the reasoning. This PR merged #335 merges Closed rather than repointed, deliberately: this PR's Nothing is lost. The three #307 boxes invalidated when |
5 participants
This PR creates the alpha 4 release merge commit
Greptile Summary
This PR merges multiple feature branches for the alpha 4 release, consolidating 19 commits with significant enhancements across game installation management, content provider infrastructure, UI improvements, and update mechanisms.
Key Changes
ManualInstallationStorageservice and comprehensive diagnostic logging throughoutGameInstallationServiceProviderDefinitionLoader,ContentPipelineFactory) enabling dynamic content source configurationGeneralsOnlineProfileReconcilerservice for syncing profile metadata and automatic update detectionSetupWizardServiceand supporting UIVelopackUpdateManagerwith artifact builds support, PR/branch subscriptions, and development channel updatesAddLocalContentViewModelwith file tree browsing for importing local mods/mapsGameSettingsMapperwith improved persistence handlingVersionComparerhelper for semantic versioning across different format conventionsTesting & Documentation
build-release.ps1scriptConfidence Score: 4/5
GameInstallationService.InvalidateCache()whereGetAwaiter().GetResult()is called while holding a synchronous lock. Otherwise, the changes demonstrate solid engineering practices with proper async/await patterns, resource disposal, caching strategies, and diagnostic logging throughout.GameInstallationService.cs:227where the synchronous lock with async call could cause threading issuesImportant Files Changed
Sequence Diagram
sequenceDiagram participant User participant UI as UI/ViewModel participant InstallService as GameInstallationService participant ManualStorage as ManualInstallationStorage participant Detector as InstallationDetector participant ContentFactory as ContentPipelineFactory participant ProviderLoader as ProviderDefinitionLoader participant UpdateManager as VelopackUpdateManager Note over User,UpdateManager: Manual Installation Flow (New Feature) User->>UI: Select installation folder UI->>InstallService: RegisterManualInstallationAsync() InstallService->>ManualStorage: SaveManualInstallationAsync() ManualStorage-->>InstallService: Success InstallService->>Detector: DetectGameClients() Detector-->>InstallService: GameClients InstallService-->>UI: Installation registered Note over User,UpdateManager: Provider Configuration Loading (New Architecture) UI->>ProviderLoader: LoadProvidersAsync() ProviderLoader->>ProviderLoader: Load bundled JSON providers ProviderLoader->>ProviderLoader: Load user JSON providers (override) ProviderLoader-->>UI: Provider definitions UI->>ContentFactory: CreatePipeline(providerDef) ContentFactory->>ContentFactory: Instantiate discoverer/resolver/deliverer ContentFactory-->>UI: Content pipeline ready Note over User,UpdateManager: App Update with Artifact Support (Enhanced) User->>UI: Check for updates UI->>UpdateManager: CheckForUpdatesAsync() UpdateManager->>UpdateManager: Check GitHub releases (cached) UpdateManager-->>UI: Update available User->>UI: Check development builds UI->>UpdateManager: CheckForArtifactUpdatesAsync() UpdateManager->>UpdateManager: Query GitHub Actions artifacts UpdateManager-->>UI: Artifact available User->>UI: Install artifact UI->>UpdateManager: InstallArtifactAsync() UpdateManager->>UpdateManager: Download artifact, extract nupkg UpdateManager->>UpdateManager: Start local HTTP server UpdateManager->>UpdateManager: Apply update via Velopack UpdateManager-->>UI: Restart application