feat(replay-manager): add automatic replay saving#275
Conversation
f437d04 to
7c00d78
Compare
7c00d78 to
e3ad922
Compare
e3ad922 to
c0dfc65
Compare
c0dfc65 to
a3f85ba
Compare
5db7437 to
368fcbb
Compare
368fcbb to
c449ece
Compare
c449ece to
72e3d76
Compare
72e3d76 to
9400ab0
Compare
9400ab0 to
4e4aa51
Compare
4e4aa51 to
e0df3d7
Compare
- Remove blank lines after opening braces (SA1505) - Add trailing commas to multi-line initializers (SA1413) - Remove multiple blank lines in a row (SA1507) - Fix multi-line parameter formatting (SA1116, SA1117) - Move static methods before instance methods (SA1204) - Change log level from Information to Debug for raw match data - Fix FileShare.Read to FileShare.ReadWrite to handle concurrent access - Fix filename truncation to account for counter suffix length - Replace File.Copy with atomic FileMode.CreateNew to prevent TOCTOU races
- Move all static helper methods before public/instance methods - Remove duplicate blank line (SA1507) - Remove duplicate static method definitions - Fixes StyleCop SA1204 warning
- Add try/catch in StartMonitoring to clean up watcher if EnableRaisingEvents throws - Move watcher cleanup before IsMonitoring check in StopMonitoring to prevent leaks - Add try/catch in Dispose loop to ensure all monitors are disposed even if one fails - Addresses Greptile review comments about resource leaks
- Constant was never used and could mislead developers - Actual truncation logic correctly calculates bounds dynamically - Prevents potential PathTooLongException from incorrect usage - Addresses Greptile review comment
- Call StopMonitoring() when watcher restart fails in OnWatcherError - Ensures IsMonitoring accurately reflects monitor capability - Prevents silent event loss when watcher becomes deaf - Allows grace-period cleanup or caller retry to handle the failure - Addresses Greptile review comment
- Use replay.GameVersion instead of SelectedTab - Ensures correct parsing when UI tab doesn't match replay's game type - Addresses kilo-code-bot critical review comment
- Change FileShare.Read to FileShare.ReadWrite in both copy attempts - Matches ReplayParserService behavior for concurrent file access - Prevents IOException when game still has replay file open - Addresses kilo-code-bot critical review comments
- Wrap StartMonitoring in try/catch to remove session on failure - Prevents orphaned sessions with IsMonitoring=false in _activeSessions - Separate source file open from destination collision retry logic - Source file errors now propagate immediately instead of retrying - Reset stream position for retry attempts - Improves error diagnostics and prevents unnecessary retries - Addresses Greptile and kilo-code-bot review comments
- Add beginTimestamp > 0 guard to duration calculation - Prevents invalid durations like 485277h when beginTimestamp=0 - Mirrors the gameDate guard logic
…fix stream position drift in ReplayParserService
…r error notifications, use size-only file monitoring
…ling, FileShare.Read, cleanup on failures, stop monitoring only on success
f688168 to
0852f89
Compare
… and fix StyleCop warnings
|
https://github.com/openclaw implement extensible genhub:// URI protocol handler for deep linking into GenHub features. Requirements: Design a flexible URI routing system that supports multiple actions and resources Implementation: Create a router pattern that maps resource/action combinations to handlers GenHub.Core/Interfaces/IUriProtocolHandler.cs (main handler interface) |
Summary
00000000.rep) during gameplay and automatically saves a copy with metadata-based naming (timestamp, map name, players) when the game ends..repfiles.Changes
New Files (10)
FileSystemWatcher-based monitor with file stability detection (3 consecutive checks at 2-second intervals) to detect when the game stops writing to the replay fileConcurrentDictionary, implementsIReplayMonitorServiceSaved/subdirectory with generated filenames (yyyyMMdd_HHmmss_MapName_Players.rep), handles duplicates and filename sanitizationReplayCompletedeventGenHub.Core/Models/Events/SavedReplaysDirectoryName,DefaultReplayFileName, stability check intervals,ReplayMagicBytes, minimum file size, date formatModified Files (12)
AutoSaveReplayspropertyAutoSaveReplaysin create/update flowsAutoSaveReplaysthrough the ViewModelAutoSaveReplaysis enabled and profile is not a tool profile)Tests
GameLauncherTeststo includeIReplayMonitorServicemockSharedViewModelModuleTeststo register replay manager servicesArchitecture
Test plan
Saved/directory after game ends🤖 Generated with Claude Code
🤖 OpenClaw Changes
Files changed: 3
Lines added: 60
Lines removed: 0
Branch:
feat/auto-save-replaysCommit:
bf6a7aa5Last updated: 2026-03-16T14:30:24.103Z
Greptile Summary
This PR adds a complete automatic replay-saving pipeline to the Replay Manager: a
FileSystemWatcher-basedReplayMonitor, aReplayMonitoringServiceorchestrator, a binary GENREP header parser, and a copy-and-rename save service — all wired into the game launch/exit lifecycle and exposed through a new per-profile UI toggle. AReplayViewerWindowis also introduced to display parsed replay metadata on demand.A significant number of issues from previous review rounds have been addressed in this iteration, including:
StartMonitoringremovedNotifyFilters.LastWritedropped in favour of size-only tracking_watcherresource leak onEnableRaisingEventsfailure fixedAddOrUpdatenull-insertion race replaced with atomicTryRemove(KVP)patternDisposeloop now continues past individual monitor failuresfinallyblock inOnReplayFileCompletedAsyncnow uses the session-ID-aware stop overloadMaxPlayersInSavedFileName/MaxFileNameLengthpromoted to named constants; deadMaxFileNameLengthBeforeExtensionremovedParseReplayCorebuild-breaking method removed;ParseReplayAsyncnow wraps inTask.RunLogInformationtoLogDebugReplayViewerWindowDataContext subscription leak fixed with_currentViewModeltrackingReplayViewerWindow.axamlIsToolProfilebindingRemaining concerns:
StopMonitoringInternal(profileId, sessionId)in the success path ofOnReplayFileCompletedAsyncis placed afterReplayCompleted?.Invoke(...), so a throwing event subscriber leaves the session permanently orphaned in_activeSessionsuntil the LaunchRegistry grace-period fires.Mkey value (a full file path, e.g.maps/alpine_assault/alpine_assault.map) is stored verbatim asMapName, producing noisy filenames and UI display (acknowledged, no resolution yet).ParsePlayerSlotsusesRemoveEmptyEntries, which can shiftStartPositionindices for sparse slot records (acknowledged, no resolution yet).Confidence Score: 3/5
_activeSessionsunder failure or exception conditions.ReplayCompletedsubscriber skips the session cleanup step. None of these cause data loss or security issues, but they represent observable incorrect behavior (staleIsMonitoringstate, confusing filenames) that users could encounter in practice.ReplayMonitoringService.cs(session lifecycle cleanup),ReplayParserService.cs(map name extraction and slot parsing), andReplaySaveService.cs(error handling in CopyWithRetry).Important Files Changed
StopMonitoringInternalis skipped if aReplayCompletedevent handler throws, orphaning the session in_activeSessions.Mkey stored verbatim, outer colon-split uses RemoveEmptyEntries.Sequence Diagram
sequenceDiagram participant GL as GameLauncher participant RMS as ReplayMonitoringService participant RM as ReplayMonitor participant FSW as FileSystemWatcher participant RSS as ReplaySaveService participant RPS as ReplayParserService participant LR as LaunchRegistry GL->>RMS: StartMonitoringAsync(profileId, gameType) RMS->>RM: new ReplayMonitor() RMS->>RM: StartMonitoring(replayFilePath) RM->>FSW: EnableRaisingEvents = true (Size filter only) Note over RM,FSW: Waits for game to write to 00000000.rep FSW-->>RM: OnFileChanged (size changed) RM->>RM: StartStabilityCheck() (2s timer) loop 3 consecutive stable checks RM->>RM: CheckFileStability() end RM->>RM: StopMonitoring() RM-->>RMS: FileCompleted event RMS->>RSS: SaveReplayAsync(filePath, gameType) RSS->>RPS: ParseReplayAsync(filePath, gameType) RPS-->>RSS: ReplayMetadata RSS->>RSS: GenerateFileName(metadata) RSS->>RSS: CopyWithRetry(src, dst) RSS-->>RMS: (savedFilePath, metadata) RMS-->>RMS: ReplayCompleted event raised RMS->>RMS: StopMonitoringInternal(profileId, sessionId) Note over LR: On process exit (T=0) LR->>LR: Task.Delay(14s grace period) LR->>RMS: StopMonitoringAsync(profileId, sessionId)Comments Outside Diff (4)
GenHub/GenHub/Features/Info/ViewModels/DemoViewModelFactory.cs, line 241-244 (link)null!returned from catch bypasses null-safetyThe catch block silently swallows all exceptions and returns
null!, explicitly suppressing the compiler's null-safety analysis for a non-nullable return type. Any caller that dereferences the result without a null-check will encounter aNullReferenceExceptionat runtime with no compile-time warning.All other demo factory methods in this class fall back to a real default instance instead — for example,
CreateDemoGameSettingsViewModelcreates a freshGameSettingsViewModelin its catch.CreateDemoReplayManagershould follow the same pattern:Prompt To Fix With AI
GenHub/GenHub.Core/Interfaces/Tools/ReplayManager/IReplayMonitorService.cs, line 98-99 (link)ReplayCompletedevent has no subscribers — auto-save is silentReplayCompletedis raised inReplayMonitoringService.OnReplayFileCompletedAsyncwhen a replay is successfully saved, but no component in this PR subscribes to it. NeitherGameLaunchernorLaunchRegistryregisters a handler after callingStartMonitoringAsync. As a result, the feature ships with zero user feedback: replays are saved silently toSaved/, and the user has no way to confirm it worked (or that it happened at all) without manually inspecting the folder.The canonical fix is to subscribe to the event after starting monitoring and show a toast notification on completion:
At minimum, the lack of any subscriber for an event defined on a public interface should be called out before merge, as it leaves the feature incomplete from a UX perspective.
Prompt To Fix With AI
GenHub/GenHub/Features/Info/ViewModels/DemoViewModelFactory.cs, line 248-254 (link)Missing
ReplayParserServicein fallback constructorThe fallback
ReplayManagerViewModelinstantiation inside thecatchblock is missing theReplayParserServiceargument. The primary constructor requires 7 parameters:IReplayDirectoryServiceIReplayImportServiceIReplayExportServiceIUploadHistoryServiceReplayParserService← missing hereINotificationServiceILogger<ReplayManagerViewModel>The call as written passes
new MockNotificationService()at position 5 (which requiresReplayParserService) andnew MockLogger<>()at position 6 (which requiresINotificationService), with theloggerargument omitted entirely. This produces a compile error (CS1503 / CS7036) that would prevent the project from building.GenHub/GenHub/Features/Tools/ReplayManager/Services/ReplayMonitoringService.cs, line 242-255 (link)Orphaned session if
ReplayCompletedevent handler throwsIf any subscriber to
ReplayCompletedthrows a synchronous exception duringInvoke, the exception propagates up andStopMonitoringInternal(profileId, sessionId)at line 255 is never reached. The session remains in_activeSessionswithIsMonitoring(profileId)returningtrue, even though the underlyingReplayMonitorhas already self-stopped viaStopMonitoring()insideCheckFileStability.This mirrors the acknowledged orphaned-session risk from the
null-save-result early return, but from a different code path. The LaunchRegistry grace-period would eventually clean it up, but there's a window where the next relaunch would incorrectly attempt to stop an already-completed session.Moving
StopMonitoringInternalbefore the event raise (or wrapping it in atry/finally) ensures cleanup regardless of subscriber behaviour:Prompt To Fix All With AI
Last reviewed commit: 8f76b26