Repository navigation
Add client/server CLI on shared Plugin Manager infrastructure - #150
Jacoby6000 wants to merge 29 commits into
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Reviewed the full diff at dd57412. One Windows blocker: leaked registry input streams (inline). CLI 41/41 passes in Debug/Release; Windows CI fails cleanup. No live game/injection test.
| Assert.False((await manager.UpdateModsList()).HasErrors); | ||
| var options = ServerOptions.Parse(["server", "--chivalry2-path", _directory, "--platform", "Steam"]).ToLaunchOptions(); | ||
| var dialogue = new ConsoleDialogue(); | ||
| Assert.True((await new Chivalry2ModsInstaller(manager, dialogue).PrepareLaunch(options)).IsSome); |
There was a problem hiding this comment.
[P1] Dispose registry input streams
FileWriter.WriteAsync closes only its output, leaving local registry paks locked. Windows CI fails test cleanup and blocks packaging. Dispose owned inputs on success, failure, and cancellation; test exclusive source reopening without forced GC.
There was a problem hiding this comment.
Fixed in 97cbc03. FileWriter disposes owned inputs on success, failure, cancellation, and abandoned acquisitions. Regressions check disposal and exclusive reopening. Also locked concurrent progress aggregation. CLI 51/51 passes in Debug/Release; Windows CI passes.
There was a problem hiding this comment.
Reviewed the full diff at 3034861. Two parsing defects below; stream ownership is fixed. CLI 121/121 passes in Debug/Release; Windows CI passes. No live game/injection test.
| if (probe.GetResult(config) is { } configResult) { | ||
| ThrowErrors(probe); | ||
| return YamlConfiguration.Load(probe.GetValue(config)!, probe.CommandResult.Command.Name == "client"); |
There was a problem hiding this comment.
[P2] Ignore unrelated arguments before loading authoritative YAML
Valid YAML plus --name @missing.rsp fails response-file expansion, although YAML must replace that option. Disable expansion/validation of ignored input; add client/server regressions.
There was a problem hiding this comment.
Fixed in bd6e079. YAML selection checks only selector errors; unrelated CLI input stays ignored. Launcher response-file expansion is disabled and documented. Client/server regressions pass; CLI 129/129 in Debug/Release and Windows CI pass.
| root.Subcommands.Add(server.Command); | ||
| root.Subcommands.Add(client.Command); | ||
| var parsed = root.Parse(launcherArgs); | ||
| var definition = parsed.CommandResult.Command == server.Command ? server : client; |
There was a problem hiding this comment.
[P2] Parse launcher arguments once
Ordinary launches run both selector and root parsers; tracing shows response files opened twice. Reuse one System.CommandLine grammar and ParseResult for selection and binding.
There was a problem hiding this comment.
Fixed in bd6e079. One System.CommandLine grammar and ParseResult now handle selection and binding. Parse-count regressions confirm one call; response files are not opened. CLI 129/129 in Debug/Release and Windows CI pass.
There was a problem hiding this comment.
Reviewed the full diff at bd6e079. Prior fixes hold; inactive YAML still validates ignored content (inline). CLI 129/129 passes in Debug/Release; Windows CI passes. No live game/injection test.
| var deserializer = new DeserializerBuilder() | ||
| .WithNamingConvention(HyphenatedNamingConvention.Instance) | ||
| .WithTypeConverter(new InstallationTypeYamlConverter()) | ||
| .WithDuplicateKeyChecking() |
There was a problem hiding this comment.
[P2] Skip inactive YAML blocks
The inactive object? property still deserializes dictionaries: duplicate keys under server reject a valid client launch, and vice versa. Skip the inactive node without materializing it; keep root/shared/active validation strict. Cover nested duplicates for both commands.
There was a problem hiding this comment.
Fixed in bf3a996. A property-scoped YamlDotNet converter skips inactive nodes; root/shared/active duplicate checks remain strict. Regressions cover duplicates, tags, aliases, and malformed YAML. CLI 161/161 passes in Debug/Release; Windows CI passes.
There was a problem hiding this comment.
Reviewed the full diff at bf3a996. Prior fixes hold; CLI bypasses the GUI's headless/desync guard (inline). CLI 161/161 passes in Debug/Release; Windows CI passes. No live game/injection test.
| : base(shared, SharedLaunchOptions.Entries(settings.GameArgs, allowEmpty: true)) { | ||
| Settings = settings; | ||
| NoTui = noTui; | ||
| if (new[] { settings.GamePort, settings.PingPort, settings.QueryPort, settings.RconPort }.Distinct().Count() != 4) |
There was a problem hiding this comment.
[P2] Enforce the headless/desync guard
--headless false leaves desync enabled, unlike the GUI's compatibility guard. YAML has the same gap. Reject !Headless && DesyncPatch; require --no-desync-patch or desync-patch: false for rendering. Update examples and tests. No live crash is claimed.
There was a problem hiding this comment.
Fixed in 16c5192. Shared server validation rejects rendered launches with desync enabled and explains how to disable it. Defaults remain unchanged; clients are unaffected. CLI/YAML regressions pass: CLI 167/167 in Debug/Release and Windows CI.
There was a problem hiding this comment.
Reviewed the full diff at 16c5192. One argument-quoting defect reproduced with subprocess argv capture. CLI 167/167 passes in Debug/Release; Windows CI passes. No live game/injection test.
|
|
||
| private protected LaunchOptions BuildLaunchOptions(IEnumerable<string> prefix, LanguageExt.Option<ServerLaunchOptions> server) => new( | ||
| EnabledReleases: Shared.Mods.Distinct(), ServerBrowserBackend: Some(Shared.Backend), | ||
| LaunchArgs: string.Join(" ", prefix.Concat(GameArguments.Select(ArgumentEscaper.Escape))), |
There was a problem hiding this comment.
[P2] Preserve forwarded argument boundaries
A trailing backslash in -UserDir=C:\Server Profiles\ escapes the closing quote, absorbing subsequent launcher flags. Tabs and backslash/quote combinations also break. Reproduced through client/server CLI and YAML subprocess launches. Use vetted Windows quoting or ArgumentList; test exact argv, including independent -unchained.
There was a problem hiding this comment.
Fixed in 33f977c; fd8d2b2 only ignores probe build outputs. Shared escaping now uses pinned CliWrap ArgumentsBuilder.Escape. Added 66 subprocess argv regressions across client/server and CLI/YAML; 38 fail without the fix. CLI 233/233 passes in Debug/Release; Windows CI passes.
There was a problem hiding this comment.
Reviewed the full diff at fd8d2b2. One managed-inventory handoff defect reproduced through Core installation/signing. CLI 233/233 passes in Debug/Release; Windows CI passes. No live game/injection test.
| var cachePath = Path.Combine(options.Chivalry2Path, FilePaths.ModCachePath, "cli_mod_manager.json"); | ||
| var codec = new ModManagerCodec(registry); | ||
| ModManager? cached = null; | ||
| var existingCache = File.Exists(cachePath) ? cachePath : FilePaths.ModManagerConfigPath; |
There was a problem hiding this comment.
[P2] Share physical managed-pak ownership
After GUI → CLI → GUI → CLI, the stale CLI inventory misses later GUI paks and treats them as unmanaged. Excluded mods remain signed and installed; reordering can leave duplicates. Reconcile directory ownership independently of host selections. Test sequential handoffs while preserving unknown paks and GUI selections.
There was a problem hiding this comment.
Fixed in e05766b. Core reconciles physical ownership through .unchained_managed_paks.json; legacy caches seed it once. Host selections stay separate. Six regressions cover handoffs, duplicates, unknown files, and partial-failure retries. CLI 239/239 passes in Debug/Release; Windows CI passes. Concurrent preparation remains unsupported at this commit.
df2204b to
31c91c7
Compare
| if (!roots.Any(root => root.Org.Equals("Chiv2-Community", StringComparison.OrdinalIgnoreCase) | ||
| && root.ModuleName.Equals("UnchainedPlugin", StringComparison.OrdinalIgnoreCase))) | ||
| roots.Insert(0, new ReleaseCoordinates("Chiv2-Community", "UnchainedPlugin", "latest")); | ||
| foreach (var root in roots) Visit(root.Org + "/" + root.ModuleName, root.Version, exact: true); |
There was a problem hiding this comment.
[P2] Index explicit DLL pins before dependency traversal
Requesting a dependent before its explicitly pinned dependency can greedily select a newer version and report a false conflict; reversing the order succeeds. Resolve against all explicit pins and test both orders.
There was a problem hiding this comment.
Fixed in 9283596: index explicit pins before traversal. Ordering, conflict, target, prerelease, and cycle regressions pass; CLI 359/359 in Debug/Release.
| cancellationToken.ThrowIfCancellationRequested(); | ||
| if (updated.IsNone) return LanguageExt.Option<LaunchOptions>.None; | ||
| try { | ||
| updated = await modInstaller.PrepareLaunch(updated.IfNone(launch)); |
There was a problem hiding this comment.
[P2] Revalidate paks after DLL preparation
DLL preparation can select an older pak whose transitive dependencies were not checked earlier. Installation/signing then accepts an incomplete set. Rerun ValidateReleases after DLL preparation and before pak installation.
There was a problem hiding this comment.
Fixed in 2377958: revalidate after DLL preparation, before installation. Client/server regressions cover missing dependencies and valid older releases; CLI 370/370 passes in Debug/Release.
Cache verified paks and DLLs with per-object download leases, atomic publication, hard links, and copy fallback. Route CLI assets by saved profile and keep GUI selections shared. Start modded processes suspended for validated injection and initialize server profiles using game-normalized suffixes.
Add a client/server CLI with YAML hosting, shared verified mod caches, and safer DLL injection.
list,get, andversionsqueries.serversentry ownsmod-setup,ini, and an optionalsaved-dir-suffix(default: its key). Startup is sequential; shutdown is grouped; natural exits remain independent.{type, value}. Support scalar, enum, record, and list values.rawpasses an exact string through a CLI option instead. Validate before preparation.%LOCALAPPDATA%/Chivalry 2/Saved_<suffix>/unchained/{paks,plugins}; client suffix defaults tounchained.--additional-pak-dir; leave installation-wide files untouched and mounted.-67retry.Examples
Run on Windows with the CLI executable and an existing game installation. Replace the installation path; it must contain
TBL.These examples pin registered releases. Replace the core plugin pin with a registered build supporting
--additional-pak-dirbefore launching.Client:
Headless server:
Two servers, saved as
server.yaml:Profiles default to
firstandsecond. Each selects its own paks/DLLs and enables the Chiv2Turbo actor; matching bytes share the verified cache. Installing a pak does not enable its server actors;mod-actors/--server-moddoes. The official registry is included automatically. Server ports must be available and reachable by players.Validation and limits
--additional-pak-dir; live checks used a local build. This PR does not release the plugin change.2377958.Related: #149, #152