Adding a check if the app is steam, forcing internal path for steam c…#493
Adding a check if the app is steam, forcing internal path for steam c…#493jks211 wants to merge 3 commits intoutkarshdalal:masterfrom
Conversation
…lient reference for non-steam apps
📝 WalkthroughWalkthroughAdded an overloaded getAppDirPath(gameId: Int, steamApp: Boolean) in SteamService and updated SteamUtils to call it with a GameSource-derived flag, changing app directory selection to consider steamApp when choosing internal vs external storage paths. Changes
Sequence Diagram(s)sequenceDiagram
participant SteamUtils as SteamUtils (caller)
participant SteamService as SteamService
participant Pref as PrefManager / FS
SteamUtils->>SteamService: getAppDirPath(appId, isSteamSource)
alt legacy internal exists
SteamService->>Pref: checkInternalPath(appId, legacyNames)
Pref-->>SteamService: internalPath (exists)
SteamService-->>SteamUtils: return internalPath
else check external when steamApp true
SteamService->>Pref: checkExternalPath(appId)
Pref-->>SteamService: externalPath (exists?)
alt external exists
SteamService-->>SteamUtils: return externalPath
else fallback based on Pref.useExternalStorage
SteamService->>Pref: read useExternalStorage
Pref-->>SteamService: useExternalStorage (true/false)
alt true
SteamService-->>SteamUtils: return externalDefault
else
SteamService-->>SteamUtils: return internalPathFallback
end
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/utils/SteamUtils.kt`:
- Around line 230-233: The contains-based source check is fragile; use
ContainerUtils.extractGameSourceFromContainerId(appId) instead of
appId.contains("${GameSource.STEAM.name}") in replaceSteamclientDll so the
source is derived via prefix matching. Keep steamAppId as
ContainerUtils.extractGameIdFromContainerId(appId), call
ContainerUtils.extractGameSourceFromContainerId(appId) to get the source value,
and pass that result into SteamService.getAppDirPath(steamAppId, <source>)
rather than the boolean contains expression.
|
I realize that probably overloading this utils function isn't good architectural hygiene, I was going for a quicker fix to avoid refactoring but if there is a less invasive way of doing it properly let me know. |
| fun getAppDirPath(gameId: Int, steamApp: Boolean): String { | ||
|
|
||
| val info = getAppInfoOf(gameId) | ||
| val appName = getAppDirName(info) | ||
| val oldName = info?.name.orEmpty() | ||
|
|
||
| // Internal first (legacy installs), external second | ||
| val internalPath = Paths.get(internalAppInstallPath, appName) | ||
| if (Files.exists(internalPath)) return internalPath.pathString | ||
| val internalOld = Paths.get(internalAppInstallPath, oldName) | ||
| if (oldName.isNotEmpty() && Files.exists(internalOld)) return internalOld.pathString | ||
|
|
||
| val externalPath = Paths.get(externalAppInstallPath, appName) | ||
| val externalOld = Paths.get(externalAppInstallPath, oldName) | ||
| if (steamApp) { | ||
| if (Files.exists(externalPath)) return externalPath.pathString | ||
| if (oldName.isNotEmpty() && Files.exists(externalOld)) return externalOld.pathString | ||
| } | ||
|
|
||
| // Nothing on disk yet – default to whatever location you want new installs to use | ||
| if (PrefManager.useExternalStorage && steamApp) { | ||
| return externalPath.pathString | ||
| } | ||
| return internalPath.pathString | ||
| } |
There was a problem hiding this comment.
Is this a copy of the getAppDirPath method, with an extra param?
if (steamApp) {
if (Files.exists(externalPath)) return externalPath.pathString
if (oldName.isNotEmpty() && Files.exists(externalOld)) return externalOld.pathString
}
I see only that changed. Why only if it's a steam app?
There was a problem hiding this comment.
I had to really dive deeper to make sure this was the "simplest fix". There is a sequence of backing up steam client dlls and restoring them for each game that launches. It happens regardless if the game is EPIC, GOG, Steam, etc.
I didn't want to just assume we can skip this backup/restore step altogether, I would suspect that the maybe the steam client can be used for steaminput even in non steam games and/or for other future features.
Take a look at launchApp in MainViewModel.kt (line 250).
The other option is we give a bogus name for the the variable appName, and thus the rootdirectory for the recursive search (the variable appName is an empty string in cases of non steam games). That way the recursive search for a steam client fails immediately... that didn't sit right for me as a fix either.
Merging latest master into forked-branch
3 participants
…lient reference for non-steam apps
Summary by cubic
Fixes app directory selection for steamclient DLL by detecting Steam vs non-Steam from the container source. Non-Steam apps always use the internal path; Steam apps use external storage only when enabled or already present.
Written for commit 902eeda. Summary will update on new commits.
Summary by CodeRabbit