feat: Executable Path dropdown — PICS launch entries, .bat/.cmd, URI schemes#1459
Open
playday3008 wants to merge 11 commits into
Open
feat: Executable Path dropdown — PICS launch entries, .bat/.cmd, URI schemes#1459playday3008 wants to merge 11 commits into
playday3008 wants to merge 11 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLaunchInfo adds an arguments field; Steam launch info integrates with container executable discovery (including .bat/.cmd and URI schemes), the ExecutablePathDropdown is refactored to merge Steam entries and scanned executables, XServer launch commands branch by executable type, and STEAM_Fix_400 is removed. ChangesSteam Launch Integration with Container Executables
Game Fix Cleanup
Sequence DiagramsequenceDiagram
participant UI as ContainerConfigDialog
participant SteamSvc as SteamService
participant ContainerUtils as ContainerUtils
participant XServer as XServerScreen
UI->>SteamSvc: request window launch pics (steamAppId)
SteamSvc->>UI: return LaunchInfo(executable, arguments)
UI->>ContainerUtils: scan A: drive executables
ContainerUtils->>UI: return scanned executables (.exe/.bat/.cmd)
UI->>XServer: launch selection (executablePath, execArgs)
XServer->>XServer: build command by type (URI / batch / exe)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/debug-build.yml:
- Around line 43-45: Replace the fragile sed-based injection of
applicationIdSuffix into app/build.gradle.kts with a Gradle property approach:
stop using the sed line that edits the debug block and instead pass the suffix
via the Gradle CLI (e.g., ./gradlew -PapplicationIdSuffix=.dev) or configure a
project property that the build script reads (reference applicationIdSuffix in
build.gradle.kts); additionally add a verification step after the build script
invocation that checks app/build.gradle.kts or the resulting APK/manifest for
the applied applicationIdSuffix (e.g., grep for 'applicationIdSuffix = ".dev"'
and exit non-zero if not found) so failures are detected instead of silently
ignored.
- Around line 50-54: Sanitize the branch name before using it in the artifact
name to avoid invalid characters: add a step (e.g., id: sanitize) that reads
inputs.branch, replaces any characters not matching [A-Za-z0-9._-] with a safe
character (e.g., '-') and writes the result to GITHUB_OUTPUT as branch_name,
then change the Upload debug APK step (uses: actions/upload-artifact@v4) to use
name: app-debug-${{ steps.sanitize.outputs.branch_name }} instead of directly
using inputs.branch.
- Around line 17-19: The checkout step using actions/checkout@v4 currently
leaves the GITHUB_TOKEN persisted in the workspace; update the checkout step
(the actions/checkout@v4 usage) to explicitly set persist-credentials: false so
the token is not written to the workspace or available to subsequent steps,
ensuring credential persistence is disabled while still passing the intended ref
input.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`:
- Around line 1265-1267: The code currently auto-selects the default PICS entry
when pics.isNotEmpty() && containerData.execArgs.isEmpty(), which wrongly treats
empty execArgs as "no prior selection" and can overwrite a valid saved choice
that simply has no arguments; change the guard to detect absence of a saved
executable instead (e.g., check containerData.execExecutable/isBlank or a
dedicated selected flag) so that onLaunchOptionSelected(defaultEntry.executable,
defaultEntry.arguments) is only called when there truly is no saved selection,
leaving an explicitly chosen executable with empty arguments untouched.
In `@app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt`:
- Around line 262-266: The current onLaunchOptionSelected handler retains
previous Steam-only execArgs when args is null; update the assignment in the
state.config.value = config.copy(...) call so that execArgs is cleared when args
is null (e.g., execArgs = args ?: "" or null depending on downstream
expectations) instead of using config.execArgs, referencing
onLaunchOptionSelected, state.config, config.copy, executablePath and execArgs
to locate the change.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 3727-3731: The URI branch in the when that builds the command for
executablePath (ContainerUtils.isUriScheme(executablePath)) currently emits
unquoted "start $executablePath" and uses workingDir computed via
executablePath.substringBeforeLast("/"), producing invalid commands and paths
for URIs; change the when branch to produce a wrapped Windows start invocation
like cmd /c start "" "<executablePath>" (ensuring the URI is quoted) and ensure
workingDir is chosen from the app/game base directory instead of
substringBeforeLast("/"); move the workingDir computation after the URI check
(or special-case it) so ContainerUtils.isUriScheme(executablePath) returns a
valid workingDir (e.g., app/game base) and apply the same fix in the other
occurrence referenced (around the second when at the later lines).
In `@app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.kt`:
- Around line 5-6: The test fails to compile because
kotlinx.serialization.decodeFromString is not imported; add the missing import
for kotlinx.serialization.decodeFromString so calls like
json.decodeFromString<LaunchInfo>(...) in LaunchInfoSerializationTest (used at
the decode sites) resolve correctly; update the import list alongside
encodeToString and Json to include decodeFromString.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98a4ed51-8a5f-4286-8b88-1fbda414bfb8
📒 Files selected for processing (15)
.github/workflows/debug-build.ymlapp/src/main/java/app/gamenative/data/LaunchInfo.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_400.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.ktapp/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/app/gamenative/utils/KeyValueUtils.ktapp/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.ktapp/src/test/java/app/gamenative/utils/ContainerUtilsExecutableTest.kt
💤 Files with no reviewable changes (2)
- app/src/main/java/app/gamenative/gamefixes/STEAM_400.kt
- app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt
…utablePath is empty
playday3008
force-pushed
the
feat/executable-path-dropdown-enhancement
branch
from
364ddf5 to
cb37b72
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 15 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.kt (1)
5-6:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
decodeFromStringimport breaks test compilation.Line 28 (and Lines 49, 68, 86) uses
json.decodeFromString<LaunchInfo>(...)without importingkotlinx.serialization.decodeFromString.Proposed fix
import app.gamenative.enums.OS import app.gamenative.enums.OSArch +import kotlinx.serialization.decodeFromString import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json#!/bin/bash # Verify callsites exist and import is missing/present rg -n 'decodeFromString<LaunchInfo>\(' app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.kt rg -n 'import kotlinx\.serialization\.decodeFromString' app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.ktAlso applies to: 28-28, 49-49, 68-68, 86-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.kt` around lines 5 - 6, The test uses Json.decodeFromString in LaunchInfoSerializationTest but the import for kotlinx.serialization.decodeFromString is missing; add the top-level import kotlinx.serialization.decodeFromString to the test file (same place where kotlinx.serialization.encodeToString and kotlinx.serialization.json.Json are imported) so calls like json.decodeFromString<LaunchInfo>(...) at the usages in LaunchInfoSerializationTest compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt`:
- Around line 1242-1245: The launch-path input was made read-only and its
onValueChange removed, removing users' ability to manually enter an
executable/URI; restore a manual override by making the launch path TextField
editable again and wiring its onValueChange to update local state, while still
keeping scanning/PICS suggestions and the existing onLaunchOptionSelected flow
in ContainerConfigDialog so users can type or paste an .exe, script, or URI when
detection misses a valid entry (look for the TextField or state holding the
launch path in ContainerConfigDialog and re-enable its setter/update callback).
---
Duplicate comments:
In `@app/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.kt`:
- Around line 5-6: The test uses Json.decodeFromString in
LaunchInfoSerializationTest but the import for
kotlinx.serialization.decodeFromString is missing; add the top-level import
kotlinx.serialization.decodeFromString to the test file (same place where
kotlinx.serialization.encodeToString and kotlinx.serialization.json.Json are
imported) so calls like json.decodeFromString<LaunchInfo>(...) at the usages in
LaunchInfoSerializationTest compile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a1a63c3-1151-46d7-947b-de415459b08c
📒 Files selected for processing (14)
app/src/main/java/app/gamenative/data/LaunchInfo.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_400.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.ktapp/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/app/gamenative/utils/KeyValueUtils.ktapp/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.ktapp/src/test/java/app/gamenative/utils/ContainerUtilsExecutableTest.kt
💤 Files with no reviewable changes (2)
- app/src/main/java/app/gamenative/gamefixes/STEAM_400.kt
- app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 3731-3734: The URI branch builds a detached Windows START command
causing premature termination; update the
ContainerUtils.isUriScheme(executablePath) branch to use a synchronous START
(add the /WAIT flag) so the guest launcher waits for the protocol handler to
exit, and apply the same fix to the other identical branch around the 3789–3792
region; ensure the code paths constructing the command string for
executablePath/normalizedPath use "cmd /c start /WAIT" instead of "cmd /c start"
so startExitWatchForUnmappedGameWindow() and gameTerminationCallback see the
real lifecycle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da252f27-05a5-4dd2-be69-721f7765513d
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/test/java/app/gamenative/data/LaunchInfoSerializationTest.ktapp/src/test/java/app/gamenative/utils/ContainerUtilsExecutableTest.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 3731-3734: Batch-script launches built in the when-branch (using
ContainerUtils.isBatchScript and the produced string for
executablePath/normalizedPath) break when container.execArgs is appended because
cmd /c "script.bat" <args> misparses paths; change the batch-script branch to
invoke the script via cmd /c call so the command becomes cmd /c call
"A:\${normalizedPath}" (or equivalent with proper quoting around the script
path) to preserve spaces/special characters and ensure correct batch-file
context when container.execArgs is appended. Ensure the same fix is applied to
the other occurrence around lines 3789-3792.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6ceb83e-70c8-4a0f-b0e3-054b80f87910
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
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.
Description
.bat/.cmdfiles and URI schemes (link2ea://, etc.) in dropdown, scanner, and launch pipelineargumentsfield intoLaunchInfo(no Room migration needed)LaunchArgFix— PICS already provides-game portalRecording
Screen_Recording_20260520_111009_GameNative_1.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds Steam PICS launch options to the Executable Path dropdown and supports .bat/.cmd files and URI schemes end-to-end. Improves default selection, argument handling, quoting, and reliable launch behavior for Steam and non-Steam titles.
New Features
.bat/.cmdand URI schemes across dropdown, scanner, PICS filter, and launch pipeline (usecmd /c callfor batch,start /WAITfor URIs, URI-safe quoting, skip fullscreen WM class for URIs).argumentsintoLaunchInfo(defaults to empty) to launch with correct flags..bat/.cmdand ranks.exeabove scripts.LaunchArgFix(PICS already provides-game portal).Bug Fixes
start /WAITfor URIs; invoke batch scripts viacmd /c callto handle paths with spaces.Written for commit d22a1de. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Tests