fix: address all 10 code-review findings#15
Closed
gabrielgad wants to merge 7 commits into
Closed
Conversation
… isolation) From the high-effort workflow review of the branch — all findings resolved: Crashes / data loss: - util.rs clear_tmp(): reap only dead-pid orphan scratch instead of rm -rf'ing the whole tmp/ tree (could delete through a concurrent live session's overlay into the player's save upperdir). - steamless.rs has_steamstub_elf(): bounds-check the packer-controlled ELF section name offset so a bad offset returns 'not DRM' instead of panicking the launch. Launch correctness (this session's gamescope-bypass + Keen work): - execute.rs no_gamescope: gate on the resolved seat-list emptiness (matching build_cmds) not inst.together, so a failed together seat can't make the WM wait for a gamescope window that was never built and kill the game. - keen.rs extra_launch_args(): skip --keenonline-server-data-file when the sideloaded emu binary is absent (lockstep with start_services' guard). Concurrency isolation: - standalone.rs: overlay under the per-launch namespaced tmp dir. - keen.rs data_file_path(): per-launch namespaced (loopback port still emu-side). - proton.rs get_prefix_path(): disambiguate two same-profile instances in one launch with a -p<n> suffix (first keeps its bare per-profile prefix) so Proton's prefix file-lock can't block the 2nd from booting. Window-manager layout: - niri.rs fullscreen: always honor the assigned monitor; never auto-spread onto another output (could land on a virtual Sunshine display → wedge). Removed the now-dead displays_assigned plumbing. - niri.rs tile_windows_on_monitor: index the original window snapshot instead of re-fetching + stale-indexing. - layout.rs Grid: honor window_count instead of a fixed 2x2.
Gamepad-type seats used to be the only ones getting the seat pad wired (a phantom-controller precaution) which left kbm-type together seats padless in-game. All input now goes to all together seats.
disable_gamescope used to gate on the launch as a whole (single lone local seat only). Decide it per instance instead, keyed on monitor sharing: a local instance alone on its own monitor bypasses the nested gamescope (kills the double-compositor scan-line artifact on high-refresh panels) even when the same launch also runs a together instance elsewhere, which keeps gamescope for its PipeWire capture. Window management follows: a bypassed instance is a native host-compositor surface, so the niri backend resolves window->instance ownership through the per-instance systemd scope (splitux-<ns>-g<game>-i<idx>.scope) instead of trusting list order, relaxes matching to scoped-ownership, and treats placement as best-effort rather than hard-aborting the launch.
keen-emu failures (e.g. AddrInUse from a stale port squatter) died silently behind Stdio::null — surface them in the launch log.
Handle top-level help/version before scope re-exec and monitor probing; subcommand help stays with clap.
Moves splitux's self-hosted CI jobs off the shared bare-metal `splitux` runner fleet onto dedicated, ephemeral, per-job podman containers (label splitux-ci) - Phase 2 of containerizing the org's runner fleet (following splitux-together's Phase 1 pilot). gamescope-splitux and gbe_fork-splitux's Linux jobs keep using the shared `splitux` label, unaffected. Design/rationale in ~/ssh-workspace/ci-runners/README.md on the lab.
Collaborator
Author
|
Landed via fast-forward merge to main (671e113) - only remaining delta was the CI runner label change, no conflicts. |
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
1 participant
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.
From a high-effort multi-agent review of the branch — all 10 verified findings resolved.
Crashes / data loss:
clear_tmp()reaps only dead-pid orphan scratch (no longer rm-rf's through a concurrent live session's overlay into player saves);has_steamstub_elf()bounds-checks the ELF offset instead of panicking.Launch correctness:
no_gamescopegated on the resolved seat list (matchingbuild_cmds) notinst.together; Keen skips its launch arg when the emu binary is absent.Concurrency isolation: standalone overlay + Keen data file namespaced per-launch; same-profile instances get distinct Proton prefixes.
Layout: fullscreen always honors the assigned monitor (never auto-spreads onto a virtual Sunshine output → wedge); tiling indexes the original window snapshot; Grid honors
window_count.Builds, 0 clippy warnings, 153 tests pass.