Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial support for Age of Empires IV as a replay-file telemetry source, and exposes it in the demo UI and documentation.
Changes:
- Added
AgeOfEmpires4ReplayFileSource(file-watcher source + default replay path). - Registered AoE IV in the WPF demo’s built-in file watcher sources.
- Updated README supported-games matrix and adjusted test data to skip the new wildcard-pattern source.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| README.md | Adds AoE IV to the supported games table. |
| GamesDat/Telemetry/Sources/AgeOfEmpires4/AgeOfEmpires4ReplayFileSource.cs | Introduces an AoE IV replay folder watcher with default path + wildcard pattern. |
| GamesDat.Tests/Helpers/FileWatcherTestData.cs | Adds AoE IV to an ignore list for parameterized file-watcher tests. |
| GamesDat.Demo.Wpf/ViewModels/FileWatcherTabViewModel.cs | Adds AoE IV as a built-in source in the demo UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Threading.Tasks; |
There was a problem hiding this comment.
This file includes several unused using directives (System.Collections.Generic, System.Linq, System.Text, System.Threading.Tasks). Removing them will reduce noise and avoid analyzer warnings.
| using System.Collections.Generic; | |
| using System.Linq; | |
| using System.Text; | |
| using System.Threading.Tasks; |
| try | ||
| { | ||
| var aoe4Path = AgeOfEmpires4ReplayFileSource.GetDefaultReplayPath(); | ||
| System.Diagnostics.Debug.WriteLine($"AoE IV replay path: {aoe4Path}"); | ||
| var aoe4Source = new FileWatcherSourceViewModel( | ||
| "Age of Empires IV", | ||
| aoe4Path, | ||
| "*.*", | ||
| () => new AgeOfEmpires4ReplayFileSource()); | ||
| aoe4Source.DetectedFiles.CollectionChanged += OnSourceFilesChanged; | ||
| Sources.Add(aoe4Source); | ||
| } | ||
| catch (DirectoryNotFoundException) | ||
| { |
There was a problem hiding this comment.
The new AoE IV block relies on catching DirectoryNotFoundException, but GetDefaultReplayPath() only builds a path string and won’t throw. As a result, the “(Not Installed)” source will never be added; consider checking Directory.Exists(aoe4Path) (or using the same PathExists logic used by FileWatcherSourceViewModel) instead of try/catch if you want to present an installed/not-installed state.
| var defaultPath = Path.Combine( | ||
| Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments), | ||
| "Age of Empires IV", "playback"); | ||
| var aoe4Source = new FileWatcherSourceViewModel( | ||
| "Age of Empires IV (Not Installed)", | ||
| defaultPath, | ||
| "*.*", | ||
| null); |
There was a problem hiding this comment.
The fallback defaultPath in the AoE IV catch block does not match AgeOfEmpires4ReplayFileSource.GetDefaultReplayPath() (it omits the "My Games" segment). If this path is shown to users, it will be incorrect; reuse GetDefaultReplayPath() or build the same path segments here for consistency.
| { | ||
|
|
||
| /// <summary> | ||
| /// Sources we currently explicitly ignore during testing ebcause they use an |
There was a problem hiding this comment.
Typo in XML doc: “ebcause” should be “because”.
| /// Sources we currently explicitly ignore during testing ebcause they use an | |
| /// Sources we currently explicitly ignore during testing because they use an |
No description provided.