feat(#6): Auto-detect game installation directory#46
Conversation
- Create IGameDirectoryScanner interface for game directory scanning - Implement GameDirectoryScanner with Steam/GOG/Epic detection - Support cross-platform paths (Windows, Linux, macOS) - Add comprehensive unit tests - Register service in DI container Closes #6
There was a problem hiding this comment.
Pull request overview
Adds an auto-detection service to locate Mount & Blade II: Bannerlord installation directories across common launchers/platforms, and wires it into the UI via DI.
Changes:
- Introduced
IGameDirectoryScanner+ DTO (GameDirectoryInfo) for reporting detected installs - Implemented
GameDirectoryScannerwith Steam/GOG/Epic path heuristics and basic validation/version reading - Added xUnit tests and registered the scanner in the UI DI container
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| BannerlordModEditor.UI/App.axaml.cs | Registers the new directory scanner service in DI |
| BannerlordModEditor.Common/Services/IGameDirectoryScanner.cs | Defines scanning API and the GameDirectoryInfo model |
| BannerlordModEditor.Common/Services/GameDirectoryScanner.cs | Implements multi-launcher directory detection + validation/version parsing |
| BannerlordModEditor.Common.Tests/Services/GameDirectoryScannerTests.cs | Adds unit tests for validation/version parsing and scan behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var trimmed = line.Trim(); | ||
| if (trimmed.StartsWith("\"path\"", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var parts = trimmed.Split('"', StringSplitOptions.RemoveEmptyEntries); | ||
| if (parts.Length >= 4) | ||
| { | ||
| var path = parts[3].Replace("\\\\", "\\"); | ||
| if (Directory.Exists(path)) | ||
| { | ||
| paths.Add(path); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
libraryfolders.vdf lines like "path" "D:\\SteamLibrary" will typically split into 2 tokens (path, D:\\SteamLibrary) with the current Split('"', RemoveEmptyEntries) usage, so parts.Length >= 4 will never be satisfied and Steam libraries from the VDF won’t be discovered. Consider parsing the VDF with a more robust approach (e.g., regex to capture the second quoted string, or a minimal VDF parser) and extracting the value token directly.
| catch | ||
| { | ||
| } |
There was a problem hiding this comment.
Swallowing all exceptions silently makes detection failures hard to diagnose (e.g., malformed VDF, permission issues). At minimum, catch specific exceptions and either log/debug-trace them or include a comment explaining why it’s safe to ignore; otherwise troubleshooting “why auto-detect doesn’t work” becomes very difficult.
| /// ModuleData目录路径 | ||
| /// </summary> | ||
| public string ModuleDataPath { get; set; } = string.Empty; |
There was a problem hiding this comment.
ModuleDataPath (and its comment) is misleading: the implementation sets this to the Modules directory, not a ModuleData directory. Since this is a public DTO, it will leak into consumers and be confusing; rename to something accurate like ModulesPath (and update the XML comment accordingly).
| /// ModuleData目录路径 | |
| /// </summary> | |
| public string ModuleDataPath { get; set; } = string.Empty; | |
| /// Modules目录路径 | |
| /// </summary> | |
| public string ModulesPath { get; set; } = string.Empty; |
| { | ||
| public class GameDirectoryScanner : IGameDirectoryScanner | ||
| { | ||
| private const string BannerlordAppId = "261550"; |
There was a problem hiding this comment.
BannerlordAppId is currently unused in this implementation, which adds noise and suggests incomplete/abandoned logic. Either remove it, or use it to improve Steam/Epic detection (e.g., by correlating app manifests) so the constant has a clear purpose.
| private const string BannerlordAppId = "261550"; |
| [Fact] | ||
| public async Task ScanForGameDirectoriesAsync_ReturnsEmptyListWhenNoGameInstalled() | ||
| { | ||
| var result = await _scanner.ScanForGameDirectoriesAsync(); | ||
| Assert.NotNull(result); | ||
| } |
There was a problem hiding this comment.
This test name claims the result is empty “when no game installed”, but it only asserts NotNull and also depends on the machine’s real Steam/GOG/Epic state (making it non-deterministic and potentially slow). To make it a real unit test, inject filesystem/environment dependencies (or an abstraction) and assert the expected contents (e.g., empty list) using a controlled fake setup.
Summary
IGameDirectoryScannerinterface for game directory scanningGameDirectoryScannerwith Steam/GOG/Epic detectionChanges
BannerlordModEditor.Common/Services/IGameDirectoryScanner.cs- Interface definitionBannerlordModEditor.Common/Services/GameDirectoryScanner.cs- ImplementationBannerlordModEditor.Common.Tests/Services/GameDirectoryScannerTests.cs- Unit testsBannerlordModEditor.UI/App.axaml.cs- DI registrationCloses #6