-
Notifications
You must be signed in to change notification settings - Fork 20
feat(replay-manager): add automatic replay saving #275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Changes from 28 commits
53fab46
0ea7d19
b4087d6
4fd49e0
9218ebf
5c79ddc
66d649a
6831c8c
2816049
320d6c5
12b37f6
c2c8e53
0f68bfd
f375c9f
cbfd716
6aa1fd4
7373719
cc16a81
95cb670
cdf537c
408ae39
9e914d6
7a57382
a5c90ba
38dbd11
d320b9e
5dc985f
0852f89
8f76b26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| using GenHub.Core.Models.Enums; | ||
| using GenHub.Core.Models.Events; | ||
|
|
||
| namespace GenHub.Core.Interfaces.Tools.ReplayManager; | ||
|
|
||
| /// <summary> | ||
| /// Service for monitoring replay files and automatically saving them. | ||
| /// </summary> | ||
| public interface IReplayMonitorService | ||
| { | ||
| /// <summary> | ||
| /// Event raised when a replay file has been completed and saved. | ||
| /// </summary> | ||
| event EventHandler<ReplayCompletedEventArgs>? ReplayCompleted; | ||
|
|
||
| /// <summary> | ||
| /// Starts monitoring for a specific profile. | ||
| /// </summary> | ||
| /// <param name="profileId">The profile ID.</param> | ||
| /// <param name="gameType">The game type.</param> | ||
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task StartMonitoringAsync(string profileId, GameType gameType); | ||
|
|
||
| /// <summary> | ||
| /// Stops monitoring for a specific profile. | ||
| /// </summary> | ||
| /// <param name="profileId">The profile ID.</param> | ||
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task StopMonitoringAsync(string profileId); | ||
|
|
||
| /// <summary> | ||
| /// Stops all active monitoring. | ||
| /// </summary> | ||
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task StopAllMonitoringAsync(); | ||
|
|
||
| /// <summary> | ||
| /// Stops monitoring for a specific profile, verifying session identity. | ||
| /// </summary> | ||
| /// <param name="profileId">The profile ID.</param> | ||
| /// <param name="sessionId">If provided, only stops if the active session matches this ID.</param> | ||
| /// <returns>A task representing the asynchronous operation.</returns> | ||
| Task StopMonitoringAsync(string profileId, string? sessionId); | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether monitoring is active for a profile. | ||
| /// </summary> | ||
| /// <param name="profileId">The profile ID.</param> | ||
| /// <returns>True if monitoring is active; otherwise, false.</returns> | ||
| bool IsMonitoring(string profileId); | ||
|
|
||
| /// <summary> | ||
| /// Gets the session ID for an active monitoring session. | ||
| /// </summary> | ||
| /// <param name="profileId">The profile ID.</param> | ||
| /// <returns>The session ID, or null if not monitoring.</returns> | ||
| string? GetSessionId(string profileId); | ||
| } | ||
|
Comment on lines
+9
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider extending The implementation manages 🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged. This is a valid suggestion for future improvement. For now, the implementation uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
✏️ Learnings added
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged. Valid suggestion for future improvement. Currently using IDisposable consistently with other GenHub services. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🧠 Learnings used |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| using GenHub.Core.Models.Tools.ReplayManager; | ||
|
|
||
| namespace GenHub.Core.Models.Events; | ||
|
|
||
| /// <summary> | ||
| /// Event arguments for replay completion. | ||
| /// </summary> | ||
| public sealed class ReplayCompletedEventArgs : EventArgs | ||
| { | ||
| /// <summary> | ||
| /// Gets the profile ID. | ||
| /// </summary> | ||
| public required string ProfileId { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the saved replay file path. | ||
| /// </summary> | ||
| public required string SavedFilePath { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the replay metadata. | ||
| /// </summary> | ||
| public required ReplayMetadata Metadata { get; init; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| namespace GenHub.Core.Models.Events; | ||
|
|
||
| /// <summary> | ||
| /// Event arguments for replay file completion. | ||
| /// </summary> | ||
| public sealed class ReplayFileCompletedEventArgs : EventArgs | ||
| { | ||
| /// <summary> | ||
| /// Gets the completed file path. | ||
| /// </summary> | ||
| public required string FilePath { get; init; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| namespace GenHub.Core.Models.Tools.ReplayManager; | ||
|
|
||
| /// <summary> | ||
| /// Information about a player in a replay. | ||
| /// </summary> | ||
| public sealed class PlayerInfo | ||
| { | ||
| /// <summary> | ||
| /// Gets the player name. | ||
| /// </summary> | ||
| public required string Name { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the player type (Human or AI). | ||
| /// </summary> | ||
| public required PlayerType Type { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the faction/side (e.g., USA, China, GLA). | ||
| /// </summary> | ||
| public string? Faction { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the team number. | ||
| /// </summary> | ||
| public int? Team { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the player color. | ||
| /// </summary> | ||
| public string? Color { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the AI difficulty (for AI players). | ||
| /// </summary> | ||
| public string? AiDifficulty { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the starting position/slot number. | ||
| /// </summary> | ||
| public int? StartPosition { get; init; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| namespace GenHub.Core.Models.Tools.ReplayManager; | ||
|
|
||
| /// <summary> | ||
| /// Player type enumeration. | ||
| /// </summary> | ||
| public enum PlayerType | ||
| { | ||
| /// <summary> | ||
| /// Human player. | ||
| /// </summary> | ||
| Human, | ||
|
|
||
| /// <summary> | ||
| /// AI/Computer player. | ||
| /// </summary> | ||
| Computer, | ||
|
|
||
| /// <summary> | ||
| /// Observer/spectator. | ||
| /// </summary> | ||
| Observer, | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding
CancellationTokenparameters to async methods.None of the async methods accept a
CancellationToken. For a monitoring service that may be stopped during app shutdown, supporting cancellation is important for graceful teardown. This is especially relevant forStopAllMonitoringAsyncwhich may be called during application exit.Proposed change
📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged. Valid point for future enhancement. The current methods are synchronous returning
Task.CompletedTask, so cancellation tokens would have no practical effect yet. Will add when we introduce actual async operations (e.g., live spectating via replay sharing).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@undead2146, thank you for the explanation! That makes sense—deferring cancellation tokens until there are actual async operations to cancel is a pragmatic approach. Looking forward to seeing them added when the live spectating feature is implemented. 👍✏️ Learnings added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged. Will add CancellationToken parameters when actual async operations are introduced (e.g., live spectating).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@undead2146, understood! Thank you for confirming the approach. The decision to add CancellationToken parameters when they'll have practical benefit (with actual async operations) makes sense. 👍🧠 Learnings used