refactor: realign to SOLID — polymorphic dispatch (S via D) - #25
Merged
Conversation
…s-only registry reader
Realign with SOLID: SteamService.Discover dispatched on _options.Platform
(DiscoverLinux/DiscoverWindows as private methods) — one class owning two
platform strategies. Leverages DI to move the dispatch to polymorphic
collaborators selected once at composition.
- ISteamDiscoverer + LinuxSteamDiscoverer/WindowsSteamDiscoverer; the shared
discovery mechanics (ResolveRoot/ReadLibraries/FindDarktide/Failed) move into
a composed SteamDiscoveryCore (composition, not inheritance).
- SteamService.Discover() => _discoverer.Discover() (no per-call dispatch); the
tuple-switch status derivation stays (legitimate decision table, not dispatch).
- Discoverer selected at DI by SteamDiscoveryOptions.Platform (not runtime OS) —
preserves the test-injectable Platform knob so WindowsDiscoveryTests can force
Windows discovery on Linux CI.
- SteamRegistryReader: drop the OperatingSystem.IsWindows() guard (it defeated
fail-fast), annotate [SupportedOSPlatform("windows")], and register it ONLY on
Windows. On Linux a Windows-only capability is simply unavailable — resolving
it fails fast instead of silently nulling.
Behavior-preserving (discovery outputs identical). 193 tests green, 0 warnings
(CA1416 clean via the type annotation). steam.md reference updated.
…rategies Realign with SOLID: EnginseerLaunchService dispatched _platform == Windows ? LaunchWindows : LaunchLinux (+ platform-branched required-fields + a translate flag) — one class owning two platform launch strategies. - IPlatformLaunchStrategy + WindowsLaunchStrategy/LinuxLaunchStrategy. The strategy owns the spawn (via IProcessLauncher), per-platform required discovery fields, and the Z:\ path translation (Linux). Windows is direct Process.Start with no translation; Linux is `proton run` + both STEAM_COMPAT_* env + Z:\-translated args. --log-level remains intentionally omitted. - EnginseerLaunchService keeps only the platform-agnostic orchestration (Discover -> required-fields -> PrepareModRoot -> launcher-exists -> strategy.Start -> result mapping). LaunchPlatform enum folded into strategy.Name. - Strategy selected at DI by runtime OS; tests inject the strategy + a fake IProcessLauncher. Behavior-preserving (args/env/translation byte-identical). 193 tests green. enginseer-client.md reference updated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Realigns two libraries with the single-responsibility principle, leveraging the dependency-inversion principle to replace per-call
if platform then X else Ydispatch with polymorphic collaborators selected once at DI composition. Behavior-preserving. This is the SOLID audit's outcome (OS-branching was the example; the smell is any class doing type/kind/platform dispatch that belongs on a polymorphic collaborator).steam —
ISteamDiscoverersplit + Windows-only registry readerISteamDiscoverer+LinuxSteamDiscoverer/WindowsSteamDiscoverer; shared discovery mechanics (ResolveRoot/ReadLibraries/FindDarktide/Failed) factored into a composedSteamDiscoveryCore(composition, not inheritance).SteamService.Discover() => _discoverer.Discover()— no per-call dispatch. The tuple-switchstatus derivation is left untouched (legitimate decision table, not dispatch).SteamDiscoveryOptions.Platform(not the runtime OS) — this preserves the test-injectable Platform knob soWindowsDiscoveryTestscan force Windows discovery on Linux CI. (Deliberate, documented difference fromIProcessLookup, which selects by runtime OS becauseIsGameRunninghas no test-forcing requirement.)SteamRegistryReaderbecomes honestly Windows-only: theOperatingSystem.IsWindows()guard is removed (it defeated fail-fast), the class is annotated[SupportedOSPlatform("windows")](satisfies CA1416 at the type level), and it's registered only on Windows. On Linux a Windows-only capability is simply unavailable — resolving it fails fast instead of silently nulling.WindowsSteamDiscovererdepends onISteamRegistryReader;LinuxSteamDiscovererdoes not.enginseer-client —
IPlatformLaunchStrategysplitIPlatformLaunchStrategy+WindowsLaunchStrategy/LinuxLaunchStrategy. The strategy owns the spawn (viaIProcessLauncher), per-platform required discovery fields, and theZ:\translation. Windows = directProcess.Start, no translation; Linux =proton run+ bothSTEAM_COMPAT_*env +Z:\-translated args.--log-levelremains intentionally omitted.EnginseerLaunchServicekeeps only the platform-agnostic orchestration (Discover → required-fields → PrepareModRoot → launcher-exists →strategy.Start→ result mapping). TheLaunchPlatformenum is folded intostrategy.Name. Strategy selected at DI by runtime OS; tests inject the strategy + a fakeIProcessLauncher.Validation
Z:\-translation byte-identical; reviewer line-traced the logic frommaininto the new classes (verbatim relocation).dotnet build0 warnings (CA1416 clean);dotnet test193/193 (Steam 47 incl. forced-Windows discovery on Linux, EnginseerClient 28 incl. forced-Windows launch-arg assembly on Linux).ISteamService,DiscoveryResult,SteamDiscoveryOptions,IEnginseerLaunchService,LaunchResult,LaunchStatus.steam.md+enginseer-client.md(the new splits + DI blocks).One callout for log consumers
Relocating logic into new classes shifts the
ILogger<T>source context: discovery/launch log lines now emit underLinuxSteamDiscoverer/WindowsSteamDiscoverer/SteamDiscoveryCore/WindowsLaunchStrategy/LinuxLaunchStrategyinstead ofSteamService/EnginseerLaunchService. Message text, levels, and semantics are unchanged; only the category moved (unavoidable when relocating logic). No test pins the category. Flagging in case anyone filters logs by category.Out of scope (not touched, deliberately)
AllocationResolvertype-switch onModVersionPolicy— it's a 2-D decision table (shared-policy × profile-policy), not 1-D dispatch; the branching is its single responsibility. Left as-is.