From 10a712d66b8f53ab903056921092a08bdab0176b Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 14 Aug 2026 19:02:20 +0200 Subject: [PATCH] fix(launcher): close LA4 review findings --- .github/workflows/headless-portability.yml | 57 +++ .../Orchestration/ILauncherOrchestrator.cs | 7 +- .../Orchestration/LauncherExecutableSet.cs | 64 +++- .../Orchestration/LauncherOrchestrator.cs | 183 ++++++++-- .../Profiles/LauncherProfileStore.cs | 341 ++++++++++++++++-- src/AcDream.Launcher/AcDream.Launcher.csproj | 1 + src/AcDream.Launcher/MainWindow.axaml | 71 +++- src/AcDream.Launcher/MainWindow.axaml.cs | 96 +++++ src/AcDream.Launcher/Program.cs | 14 +- src/AcDream.Launcher/ViewModels/Commands.cs | 8 +- .../ViewModels/LauncherSessionRowViewModel.cs | 7 +- .../ViewModels/LauncherShellViewModel.cs | 14 +- .../ViewModels/LauncherWindowViewModel.cs | 169 ++++++++- .../LauncherExecutableSetTests.cs | 67 ++++ .../LauncherOrchestratorTests.cs | 231 +++++++++++- .../Profiles/LauncherProfileHardeningTests.cs | 171 +++++++++ .../Profiles/RosterMergeTests.cs | 60 +++ .../LauncherProjectBoundaryTests.cs | 90 +++++ .../LauncherWindowViewModelTests.cs | 114 +++++- 19 files changed, 1631 insertions(+), 134 deletions(-) create mode 100644 tests/AcDream.Launcher.Core.Tests/Orchestration/LauncherExecutableSetTests.cs create mode 100644 tests/AcDream.Launcher.Core.Tests/Profiles/LauncherProfileHardeningTests.cs diff --git a/.github/workflows/headless-portability.yml b/.github/workflows/headless-portability.yml index 6facc5ed..3fae2923 100644 --- a/.github/workflows/headless-portability.yml +++ b/.github/workflows/headless-portability.yml @@ -7,6 +7,7 @@ on: - "AcDream.slnx" - "src/AcDream.Platform/**" - "src/AcDream.Launcher.Core/**" + - "src/AcDream.Launcher/**" - "src/AcDream.Core/**" - "src/AcDream.Core.Net/**" - "src/AcDream.Content/**" @@ -17,6 +18,7 @@ on: - "src/AcDream.UI.Abstractions/**" - "tests/AcDream.Platform.Tests/**" - "tests/AcDream.Launcher.Core.Tests/**" + - "tests/AcDream.Launcher.Tests/**" - "tests/AcDream.Core.Tests/**" - "tests/AcDream.Core.Net.Tests/**" - "tests/AcDream.Content.Tests/**" @@ -33,6 +35,7 @@ on: - "AcDream.slnx" - "src/AcDream.Platform/**" - "src/AcDream.Launcher.Core/**" + - "src/AcDream.Launcher/**" - "src/AcDream.Core/**" - "src/AcDream.Core.Net/**" - "src/AcDream.Content/**" @@ -43,6 +46,7 @@ on: - "src/AcDream.UI.Abstractions/**" - "tests/AcDream.Platform.Tests/**" - "tests/AcDream.Launcher.Core.Tests/**" + - "tests/AcDream.Launcher.Tests/**" - "tests/AcDream.Core.Tests/**" - "tests/AcDream.Core.Net.Tests/**" - "tests/AcDream.Content.Tests/**" @@ -131,6 +135,59 @@ jobs: dotnet run --project src/AcDream.Headless/AcDream.Headless.csproj -c Release -- validate --config headless-k0.json if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + portable-launcher: + strategy: + fail-fast: false + matrix: + os: [windows-latest, ubuntu-latest] + runs-on: ${{ matrix.os }} + + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Install .NET 10 + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "10.0.x" + + - name: Build and test the portable launcher + shell: pwsh + run: | + dotnet build src/AcDream.Launcher/AcDream.Launcher.csproj -c Release + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + dotnet test tests/AcDream.Launcher.Tests/AcDream.Launcher.Tests.csproj -c Release + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + - name: Publish the self-contained Linux launcher + if: runner.os == 'Linux' + shell: pwsh + run: | + dotnet publish src/AcDream.Launcher/AcDream.Launcher.csproj ` + -c Release ` + -r linux-x64 ` + -o artifacts/acdream-launcher-linux-x64 + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + - name: Verify self-contained property and artifact execution + if: runner.os == 'Linux' + shell: bash + run: | + set -euo pipefail + root=artifacts/acdream-launcher-linux-x64 + self_contained=$(dotnet msbuild \ + src/AcDream.Launcher/AcDream.Launcher.csproj \ + -nologo \ + -property:RuntimeIdentifier=linux-x64 \ + -getProperty:SelfContained | tr -d '\r\n ') + test "$self_contained" = true + test -x "$root/acdream-launcher" + test ! -f "$root/acdream-launcher.dll" + DOTNET_ROOT=/definitely-not-installed \ + DOTNET_ROOT_X64=/definitely-not-installed \ + DOTNET_MULTILEVEL_LOOKUP=0 \ + "$root/acdream-launcher" --verify-publish + linux-graphical: runs-on: ubuntu-latest diff --git a/src/AcDream.Launcher.Core/Orchestration/ILauncherOrchestrator.cs b/src/AcDream.Launcher.Core/Orchestration/ILauncherOrchestrator.cs index c2003b95..e52c443f 100644 --- a/src/AcDream.Launcher.Core/Orchestration/ILauncherOrchestrator.cs +++ b/src/AcDream.Launcher.Core/Orchestration/ILauncherOrchestrator.cs @@ -18,6 +18,11 @@ public interface ILauncherOrchestrator : IDisposable LauncherCapability GetLaunchCapability(LaunchMode mode); + LauncherCapability GetAccountLaunchCapability( + string serverName, + string accountName, + LaunchMode mode); + LauncherCapability GetProbeCapability(string serverName, string accountName); void SetInstallRecord(LauncherInstallRecord? installRecord); @@ -67,7 +72,7 @@ public interface ILauncherOrchestrator : IDisposable Task LaunchAsync( string serverName, string accountName, - string characterName, + string? characterName, LaunchMode mode, CancellationToken cancellationToken = default); diff --git a/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs b/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs index c3a4fa31..a02496ce 100644 --- a/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs +++ b/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs @@ -4,20 +4,59 @@ using AcDream.Launcher.Core.Profiles; namespace AcDream.Launcher.Core.Orchestration; /// -/// Host executable paths supplied by the current installation. LA10 will -/// resolve these from the versioned app/current pointer; LA4 keeps the -/// mapping injectable and host-agnostic. +/// Resolves and validates the co-deployed graphical/headless hosts. LA10 will +/// replace the directory lookup with its versioned-current resolver; until +/// then a missing host disables the corresponding action instead of deferring +/// failure until process creation. /// -public sealed record LauncherExecutableSet( - string GraphicalHostPath, - string HeadlessHostPath, - string? WorkingDirectory = null) +public sealed class LauncherExecutableSet { + private readonly Func _fileExists; + + public LauncherExecutableSet( + string graphicalHostPath, + string headlessHostPath, + string? workingDirectory = null, + Func? fileExists = null) + { + ArgumentException.ThrowIfNullOrWhiteSpace(graphicalHostPath); + ArgumentException.ThrowIfNullOrWhiteSpace(headlessHostPath); + GraphicalHostPath = graphicalHostPath; + HeadlessHostPath = headlessHostPath; + WorkingDirectory = workingDirectory; + _fileExists = fileExists ?? File.Exists; + } + + public string GraphicalHostPath { get; } + + public string HeadlessHostPath { get; } + + public string? WorkingDirectory { get; } + + public LauncherCapability GetAvailability(LaunchMode mode) + { + string path = mode == LaunchMode.Headless + ? HeadlessHostPath + : GraphicalHostPath; + if (_fileExists(path)) + { + return LauncherCapability.Available; + } + + string host = mode == LaunchMode.Headless + ? "headless host" + : "graphical client"; + return LauncherCapability.Unavailable( + $"The co-deployed {host} is missing at '{path}'. Reinstall or update " + + "the client before launching."); + } + public LauncherProcessSpec CreatePlaySpec( LaunchMode mode, string configFilePath) { ArgumentException.ThrowIfNullOrWhiteSpace(configFilePath); + RequireAvailable(mode); return mode == LaunchMode.Headless ? new LauncherProcessSpec( @@ -33,6 +72,7 @@ public sealed record LauncherExecutableSet( public LauncherProcessSpec CreateProbeSpec(string configFilePath) { ArgumentException.ThrowIfNullOrWhiteSpace(configFilePath); + RequireAvailable(LaunchMode.Headless); return new LauncherProcessSpec( HeadlessHostPath, ["--config", configFilePath], @@ -49,4 +89,14 @@ public sealed record LauncherExecutableSet( Path.Combine(fullDirectory, "acdream-headless" + executableSuffix), fullDirectory); } + + private void RequireAvailable(LaunchMode mode) + { + LauncherCapability capability = GetAvailability(mode); + if (!capability.IsAvailable) + { + throw new LauncherOperationException( + capability.Reason ?? "The selected launcher host is unavailable."); + } + } } diff --git a/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs b/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs index fe9a426f..57d344c9 100644 --- a/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs +++ b/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs @@ -99,6 +99,12 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator return platformCapability; } + LauncherCapability executableCapability = _executables.GetAvailability(mode); + if (!executableCapability.IsAvailable) + { + return executableCapability; + } + lock (_gate) { ThrowIfDisposed(); @@ -108,6 +114,33 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator } } + public LauncherCapability GetAccountLaunchCapability( + string serverName, + string accountName, + LaunchMode mode) + { + ArgumentException.ThrowIfNullOrWhiteSpace(serverName); + ArgumentException.ThrowIfNullOrWhiteSpace(accountName); + + LauncherCapability capability = GetLaunchCapability(mode); + if (!capability.IsAvailable) + { + return capability; + } + + lock (_gate) + { + ThrowIfDisposed(); + _ = FindAccountLocked(serverName, accountName); + ManagedActivity? active = FindActiveActivityLocked(serverName, accountName); + return active is null + ? LauncherCapability.Available + : LauncherCapability.Unavailable( + $"Stop the running {active.Kind.ToString().ToLowerInvariant()} " + + "for this account before starting another activity."); + } + } + public LauncherCapability GetProbeCapability(string serverName, string accountName) { ArgumentException.ThrowIfNullOrWhiteSpace(serverName); @@ -120,6 +153,13 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator return platformCapability; } + LauncherCapability executableCapability = + _executables.GetAvailability(LaunchMode.Headless); + if (!executableCapability.IsAvailable) + { + return executableCapability; + } + lock (_gate) { ThrowIfDisposed(); @@ -254,7 +294,7 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator public Task LaunchAsync( string serverName, string accountName, - string characterName, + string? characterName, LaunchMode mode, CancellationToken cancellationToken = default) { @@ -268,12 +308,38 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator lock (_gate) { ThrowIfDisposed(); + if (FindActiveActivityLocked(serverName, accountName) is not null) + { + throw new LauncherOperationException( + "A session or character refresh is already running for this account."); + } + ServerProfile server = FindServerLocked(serverName); AccountProfile account = FindAccountLocked(serverName, accountName); - CharacterProfile character = FindCharacterLocked( - serverName, - accountName, - characterName); + CharacterProfile character; + if (string.IsNullOrWhiteSpace(characterName)) + { + if (mode != LaunchMode.GuiSelect) + { + throw new LauncherOperationException( + "Select a cached character for GUI or headless launch."); + } + + character = new CharacterProfile + { + Name = string.Empty, + LaunchMode = LaunchMode.GuiSelect, + Plugins = [], + LoginCommands = [], + }; + } + else + { + character = FindCharacterLocked( + serverName, + accountName, + characterName); + } LauncherInstallRecord install = _installRecord ?? throw new LauncherOperationException(FirstRunRequired); @@ -283,7 +349,7 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator LauncherActivityKind.Play, server.Name, account.Account, - character.Name, + string.IsNullOrWhiteSpace(characterName) ? null : character.Name, mode, "Preparing session configuration…"); _activities.Add(activity); @@ -620,9 +686,12 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator lock (_gate) { - request.Activity.State = LauncherActivityState.Cancelled; - request.Activity.Status = "Operation cancelled."; - request.Activity.Error = null; + if (!request.Activity.IsTerminal) + { + request.Activity.State = LauncherActivityState.Cancelled; + request.Activity.Status = "Operation cancelled."; + request.Activity.Error = null; + } } RaiseStateChanged(); @@ -638,9 +707,12 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator password); lock (_gate) { - request.Activity.State = LauncherActivityState.Failed; - request.Activity.Status = message; - request.Activity.Error = message; + if (!request.Activity.IsTerminal) + { + request.Activity.State = LauncherActivityState.Failed; + request.Activity.Status = message; + request.Activity.Error = message; + } } RaiseStateChanged(); @@ -681,15 +753,19 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator } break; case LauncherSessionState.Exited: - activity.ExitCode = activity.Supervisor?.ExitCode; - if (activity.State is not ( - LauncherActivityState.Failed - or LauncherActivityState.Cancelled)) + activity.ExitCode ??= activity.Supervisor?.ExitCode; + if (!activity.IsTerminal) { activity.State = LauncherActivityState.Exited; - activity.Status = activity.ExitCode is int code - ? $"Host process exited with code {code}." - : "Host process exited."; + activity.Status = activity.HostTerminalStatus + ?? (activity.ExitCode is int code + ? $"Host process exited with code {code}." + : "Host process exited."); + } + else if (activity.State == LauncherActivityState.Exited + && activity.HostTerminalStatus is not null) + { + activity.Status = activity.HostTerminalStatus; } break; } @@ -723,6 +799,27 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator return; } + if (activity.IsTerminal) + { + switch (statusEvent) + { + case ExitedStatusEvent exited: + activity.ExitCode ??= exited.Code; + activity.HostTerminalStatus ??= + $"Exited: {exited.Reason} (code {exited.Code})."; + if (activity.State == LauncherActivityState.Exited) + { + activity.Status = activity.HostTerminalStatus; + } + break; + case CharacterListStatusEvent roster: + ApplyRosterLocked(activity, roster, updateStatus: false); + break; + } + + return; + } + switch (statusEvent) { case StartedStatusEvent: @@ -762,7 +859,9 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator case ExitedStatusEvent exited: activity.State = LauncherActivityState.Exited; activity.ExitCode = exited.Code; - activity.Status = $"Exited: {exited.Reason} (code {exited.Code})."; + activity.HostTerminalStatus = + $"Exited: {exited.Reason} (code {exited.Code})."; + activity.Status = activity.HostTerminalStatus; break; case MalformedStatusEvent malformed: activity.Error = $"Malformed host status event: {malformed.Error}"; @@ -778,7 +877,8 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator private void ApplyRosterLocked( ManagedActivity activity, - CharacterListStatusEvent roster) + CharacterListStatusEvent roster, + bool updateStatus = true) { if (!string.Equals( roster.AccountName, @@ -792,19 +892,22 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator try { - _profileStore.MergeRoster( - activity.ServerName, - activity.AccountName, - roster.Characters - .Select(character => new CharacterRosterEntry( - character.Id, - character.Name, - character.SecondsGreyedOut)) - .ToArray()); - _profileStore.Save(); - activity.Status = roster.Characters.Count == 1 - ? "Character roster refreshed: 1 character." - : $"Character roster refreshed: {roster.Characters.Count} characters."; + _profileStore.ExecuteTransaction(() => + _profileStore.MergeRoster( + activity.ServerName, + activity.AccountName, + roster.Characters + .Select(character => new CharacterRosterEntry( + character.Id, + character.Name, + character.SecondsGreyedOut)) + .ToArray())); + if (updateStatus) + { + activity.Status = roster.Characters.Count == 1 + ? "Character roster refreshed: 1 character." + : $"Character roster refreshed: {roster.Characters.Count} characters."; + } } catch (Exception ex) { @@ -812,7 +915,10 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator "Could not save the refreshed character roster", ex, secret: null); - activity.Status = activity.Error; + if (updateStatus) + { + activity.Status = activity.Error; + } } } @@ -879,8 +985,7 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator lock (_gate) { ThrowIfDisposed(); - mutation(); - _profileStore.Save(); + _profileStore.ExecuteTransaction(mutation); } RaiseStateChanged(); @@ -1125,6 +1230,8 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator public string? Error { get; set; } + public string? HostTerminalStatus { get; set; } + public ILauncherProcessSupervisor? Supervisor { get; set; } public EventHandler? SupervisorStateHandler { get; set; } @@ -1140,6 +1247,8 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator or LauncherActivityState.Failed or LauncherActivityState.Cancelled); + public bool IsTerminal => !IsActive; + public LauncherSessionSnapshot ToSnapshot() => new( SessionId, diff --git a/src/AcDream.Launcher.Core/Profiles/LauncherProfileStore.cs b/src/AcDream.Launcher.Core/Profiles/LauncherProfileStore.cs index 01717400..f3a1a1b3 100644 --- a/src/AcDream.Launcher.Core/Profiles/LauncherProfileStore.cs +++ b/src/AcDream.Launcher.Core/Profiles/LauncherProfileStore.cs @@ -81,6 +81,8 @@ public sealed class LauncherProfileStore return false; } + EnsureExistingCredentialFilePermissions(); + LauncherProfileDocument? document; using (FileStream stream = File.OpenRead(FilePath)) { @@ -110,6 +112,7 @@ public sealed class LauncherProfileStore + $"expected {CurrentVersion}."); } + ValidateAndNormalizeDocument(document); Document = document; return true; } @@ -152,6 +155,13 @@ public sealed class LauncherProfileStore JsonSerializer.Serialize(stream, Document, SerializerOptions); } + if (OperatingSystem.IsLinux() + && File.GetUnixFileMode(tempPath) != OwnerOnlyFileMode) + { + throw new IOException( + "The launcher credential temp file could not be secured to mode 0600."); + } + File.Move(tempPath, FilePath, overwrite: true); } catch @@ -160,10 +170,6 @@ public sealed class LauncherProfileStore throw; } - if (OperatingSystem.IsLinux()) - { - File.SetUnixFileMode(FilePath, OwnerOnlyFileMode); - } } /// @@ -248,21 +254,21 @@ public sealed class LauncherProfileStore throw new LauncherProfileException( $"A server named '{newName}' already exists."); } - - server.Name = newName; } if (newHost is not null) { ArgumentException.ThrowIfNullOrWhiteSpace(newHost); - server.Host = newHost; } if (newPort is not null) { RequireValidPort(newPort.Value); - server.Port = newPort.Value; } + + server.Name = newName ?? server.Name; + server.Host = newHost ?? server.Host; + server.Port = newPort ?? server.Port; } public void RemoveServer(string name) @@ -308,10 +314,10 @@ public sealed class LauncherProfileStore throw new LauncherProfileException( $"Account '{newAccount}' already exists on server '{serverName}'."); } - - profile.Account = newAccount; } + profile.Account = newAccount ?? profile.Account; + if (newPassword is not null) { profile.Password = newPassword; @@ -345,6 +351,9 @@ public sealed class LauncherProfileStore IReadOnlyList? loginCommands = null) { ArgumentException.ThrowIfNullOrWhiteSpace(characterName); + RequireValidLaunchMode(launchMode); + ValidateStringList(plugins, "plugin", requireUnique: true); + ValidateStringList(loginCommands, "login command", requireUnique: false); ServerProfile server = FindServerOrThrow(serverName); AccountProfile profile = FindAccountOrThrow(server, account); @@ -393,6 +402,8 @@ public sealed class LauncherProfileStore AccountProfile profile = FindAccountOrThrow(server, account); CharacterProfile character = FindCharacterOrThrow(profile, characterName); + string? normalizedId = null; + if (newName is not null) { ArgumentException.ThrowIfNullOrWhiteSpace(newName); @@ -402,13 +413,11 @@ public sealed class LauncherProfileStore throw new LauncherProfileException( $"Character '{newName}' already exists on account '{account}'."); } - - character.Name = newName; } if (newId is not null) { - string? normalizedId = NormalizeCharacterId(newId); + normalizedId = NormalizeCharacterId(newId); if (normalizedId is not null && profile.Characters.Any(candidate => !ReferenceEquals(candidate, character) @@ -417,7 +426,19 @@ public sealed class LauncherProfileStore throw new LauncherProfileException( $"Character id '{normalizedId}' already exists on account '{account}'."); } + } + if (launchMode is not null) + { + RequireValidLaunchMode(launchMode.Value); + } + + ValidateStringList(plugins, "plugin", requireUnique: true); + ValidateStringList(loginCommands, "login command", requireUnique: false); + + character.Name = newName ?? character.Name; + if (newId is not null) + { character.Id = normalizedId; } @@ -437,6 +458,57 @@ public sealed class LauncherProfileStore } } + private void EnsureExistingCredentialFilePermissions() + { + if (!OperatingSystem.IsLinux()) + { + return; + } + + try + { + UnixFileMode mode = File.GetUnixFileMode(FilePath); + if (mode != OwnerOnlyFileMode) + { + File.SetUnixFileMode(FilePath, OwnerOnlyFileMode); + mode = File.GetUnixFileMode(FilePath); + } + + if (mode != OwnerOnlyFileMode) + { + throw new IOException($"Mode remained {mode} after normalization."); + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + throw new LauncherProfileException( + $"'{FilePath}' could not be secured to owner-only mode 0600.", + ex); + } + } + + /// + /// Applies one profile mutation and its atomic file replacement as a + /// single in-memory/on-disk transaction. Any validation or I/O failure + /// restores the exact pre-mutation document, including credentials. + /// + public void ExecuteTransaction(Action mutation) + { + ArgumentNullException.ThrowIfNull(mutation); + LauncherProfileDocument before = CloneDocument(Document); + try + { + mutation(); + ValidateAndNormalizeDocument(Document); + Save(); + } + catch + { + Document = before; + throw; + } + } + public void RemoveCharacter( string serverName, string account, @@ -472,38 +544,49 @@ public sealed class LauncherProfileStore ServerProfile server = FindServerOrThrow(serverName); AccountProfile profile = FindAccountOrThrow(server, account); + var rosterIds = new HashSet(); + var rosterNames = new HashSet(StringComparer.Ordinal); + foreach (CharacterRosterEntry entry in roster) + { + if (entry.Id == 0) + { + throw new LauncherProfileException("A roster character id cannot be zero."); + } + + ArgumentException.ThrowIfNullOrWhiteSpace(entry.Name); + if (!rosterIds.Add(entry.Id) || !rosterNames.Add(entry.Name)) + { + throw new LauncherProfileException( + "The reported character roster contains a duplicate id or name."); + } + } + foreach (CharacterRosterEntry entry in roster) { string idText = CharacterIdFormat.ToHexString(entry.Id); - // Normalize BOTH sides through TryParse/ToHexString rather - // than a raw string compare (Campaign LA plan §LA3 review - // finding F10): a stored id that round-trips to the same - // uint (different case, or — before this fix — no "0x" - // prefix) must match even though its text isn't byte- - // identical to the canonical form this method itself always - // writes. - CharacterProfile? existing = profile.Characters.Find( - character => CharacterIdFormat.TryParse(character.Id, out uint existingId) - && existingId == entry.Id); - - // Defensive fallback for a row whose id is missing OR - // unparseable (e.g. a hand-edited id with no "0x" prefix, - // which TryParse now rejects outright) — match by name - // instead so a later merge self-heals the id into the - // canonical form rather than creating a permanent duplicate - // row. - existing ??= profile.Characters.Find( - character => !CharacterIdFormat.TryParse(character.Id, out _) - && string.Equals( - character.Name, - entry.Name, - StringComparison.Ordinal)); + CharacterProfile[] matches = profile.Characters + .Where(character => + (CharacterIdFormat.TryParse(character.Id, out uint existingId) + && existingId == entry.Id) + || string.Equals(character.Name, entry.Name, StringComparison.Ordinal)) + .ToArray(); + CharacterProfile? existing = matches.FirstOrDefault(character => + CharacterIdFormat.TryParse(character.Id, out uint existingId) + && existingId == entry.Id) + ?? matches.FirstOrDefault(); if (existing is not null) { existing.Id = idText; existing.Name = entry.Name; + foreach (CharacterProfile duplicate in matches) + { + if (!ReferenceEquals(duplicate, existing)) + { + profile.Characters.Remove(duplicate); + } + } continue; } @@ -583,6 +666,192 @@ public sealed class LauncherProfileStore && CharacterIdFormat.TryParse(right, out uint rightId) && leftId == rightId; + private static LauncherProfileDocument CloneDocument( + LauncherProfileDocument source) => + new() + { + Version = source.Version, + Servers = source.Servers.Select(server => new ServerProfile + { + Name = server.Name, + Host = server.Host, + Port = server.Port, + Accounts = server.Accounts.Select(account => new AccountProfile + { + Account = account.Account, + Password = account.Password, + Characters = account.Characters.Select(character => new CharacterProfile + { + Name = character.Name, + Id = character.Id, + LaunchMode = character.LaunchMode, + Plugins = [.. character.Plugins], + LoginCommands = [.. character.LoginCommands], + }).ToList(), + }).ToList(), + }).ToList(), + }; + + private static void ValidateAndNormalizeDocument(LauncherProfileDocument document) + { + if (document.Servers is null) + { + throw new LauncherProfileException("The servers collection cannot be null."); + } + + var serverNames = new HashSet(StringComparer.Ordinal); + var normalizedIds = new List<(CharacterProfile Character, uint Id)>(); + foreach (ServerProfile? server in document.Servers) + { + if (server is null) + { + throw new LauncherProfileException("A server entry cannot be null."); + } + + RequireLoadedText(server.Name, "server name"); + RequireLoadedText(server.Host, $"host for server '{server.Name}'"); + RequireValidPort(server.Port); + if (!serverNames.Add(server.Name)) + { + throw new LauncherProfileException( + $"A server named '{server.Name}' appears more than once."); + } + + if (server.Accounts is null) + { + throw new LauncherProfileException( + $"The accounts collection for server '{server.Name}' cannot be null."); + } + + var accountNames = new HashSet(StringComparer.Ordinal); + foreach (AccountProfile? account in server.Accounts) + { + if (account is null) + { + throw new LauncherProfileException( + $"A null account appears under server '{server.Name}'."); + } + + RequireLoadedText(account.Account, "account name"); + if (account.Password is null) + { + throw new LauncherProfileException( + $"Password for account '{account.Account}' cannot be null."); + } + + if (!accountNames.Add(account.Account)) + { + throw new LauncherProfileException( + $"Account '{account.Account}' appears more than once on server '{server.Name}'."); + } + + if (account.Characters is null) + { + throw new LauncherProfileException( + $"The characters collection for account '{account.Account}' cannot be null."); + } + + var characterNames = new HashSet(StringComparer.Ordinal); + var characterIds = new HashSet(); + foreach (CharacterProfile? character in account.Characters) + { + if (character is null) + { + throw new LauncherProfileException( + $"A null character appears under account '{account.Account}'."); + } + + RequireLoadedText(character.Name, "character name"); + if (!characterNames.Add(character.Name)) + { + throw new LauncherProfileException( + $"Character '{character.Name}' appears more than once on account '{account.Account}'."); + } + + RequireValidLaunchMode(character.LaunchMode); + if (character.Id is not null) + { + if (!CharacterIdFormat.TryParse(character.Id, out uint id) || id == 0) + { + throw new LauncherProfileException( + $"Character '{character.Name}' has an invalid id '{character.Id}'."); + } + + if (!characterIds.Add(id)) + { + throw new LauncherProfileException( + $"Character id '{character.Id}' appears more than once on account '{account.Account}'."); + } + + normalizedIds.Add((character, id)); + } + + if (character.Plugins is null || character.LoginCommands is null) + { + throw new LauncherProfileException( + $"Character '{character.Name}' has a null settings collection."); + } + + ValidateStringList(character.Plugins, "plugin", requireUnique: true); + ValidateStringList( + character.LoginCommands, + "login command", + requireUnique: false); + } + } + } + + foreach ((CharacterProfile character, uint id) in normalizedIds) + { + character.Id = CharacterIdFormat.ToHexString(id); + } + } + + private static void ValidateStringList( + IReadOnlyList? values, + string valueName, + bool requireUnique) + { + if (values is null) + { + return; + } + + HashSet? seen = requireUnique + ? new HashSet(StringComparer.Ordinal) + : null; + foreach (string? value in values) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new LauncherProfileException( + $"A {valueName} cannot be null or whitespace."); + } + + if (seen is not null && !seen.Add(value)) + { + throw new LauncherProfileException( + $"The {valueName} '{value}' appears more than once."); + } + } + } + + private static void RequireLoadedText(string? value, string field) + { + if (string.IsNullOrWhiteSpace(value)) + { + throw new LauncherProfileException($"The {field} cannot be null or whitespace."); + } + } + + private static void RequireValidLaunchMode(LaunchMode mode) + { + if (!Enum.IsDefined(mode)) + { + throw new LauncherProfileException($"Launch mode '{mode}' is not supported."); + } + } + private static void RequireValidPort(int port) { if (port is < 1 or > 65535) diff --git a/src/AcDream.Launcher/AcDream.Launcher.csproj b/src/AcDream.Launcher/AcDream.Launcher.csproj index 33b07cd5..a4c20260 100644 --- a/src/AcDream.Launcher/AcDream.Launcher.csproj +++ b/src/AcDream.Launcher/AcDream.Launcher.csproj @@ -10,6 +10,7 @@ true true true + true diff --git a/src/AcDream.Launcher/MainWindow.axaml b/src/AcDream.Launcher/MainWindow.axaml index cc63bb44..f85d1e8a 100644 --- a/src/AcDream.Launcher/MainWindow.axaml +++ b/src/AcDream.Launcher/MainWindow.axaml @@ -85,7 +85,8 @@