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 1 commit intoutkarshdalal:masterfrom
Conversation
…lient reference for non-steam apps
📝 WalkthroughWalkthroughAdded a new overloaded Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
| suspend fun replaceSteamclientDll(context: Context, appId: String) { | ||
| val steamAppId = ContainerUtils.extractGameIdFromContainerId(appId) | ||
| val appDirPath = SteamService.getAppDirPath(steamAppId) | ||
| val appDirPath = SteamService.getAppDirPath(steamAppId, (appId.contains("${GameSource.STEAM.name}"))) | ||
| val container = ContainerUtils.getContainer(context, appId) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for GameSource enum definition
rg -n "enum class GameSource" --type kotlin -A 10Repository: utkarshdalal/GameNative
Length of output: 855
🏁 Script executed:
# Look at ContainerUtils helpers for source detection or ID parsing
rg -n "object ContainerUtils|class ContainerUtils" --type kotlin -A 5Repository: utkarshdalal/GameNative
Length of output: 560
🏁 Script executed:
# Search for container ID construction and format patterns
rg -n "containerId|container.*id" --type kotlin app/src/main/java | head -30Repository: utkarshdalal/GameNative
Length of output: 4541
🏁 Script executed:
# Check how source detection is done elsewhere in the codebase
rg -n "GameSource\." --type kotlin | head -40Repository: utkarshdalal/GameNative
Length of output: 4805
🏁 Script executed:
# Look at the actual function context in SteamUtils.kt around line 230-233
cat -n app/src/main/java/app/gamenative/utils/SteamUtils.kt | sed -n '220,250p'Repository: utkarshdalal/GameNative
Length of output: 1718
🏁 Script executed:
rg -n "fun extractGameSourceFromContainerId" -A 10 --type kotlinRepository: utkarshdalal/GameNative
Length of output: 1228
Use the existing extractGameSourceFromContainerId() helper for source detection.
The .contains() check is weaker than necessary. The ContainerUtils class already provides extractGameSourceFromContainerId() which properly uses prefix matching (startsWith()). Use that instead to extract the source explicitly and pass the result.
Suggested fix
suspend fun replaceSteamclientDll(context: Context, appId: String) {
val steamAppId = ContainerUtils.extractGameIdFromContainerId(appId)
- val appDirPath = SteamService.getAppDirPath(steamAppId, (appId.contains("${GameSource.STEAM.name}")))
+ val gameSource = ContainerUtils.extractGameSourceFromContainerId(appId)
+ val appDirPath = SteamService.getAppDirPath(steamAppId, (gameSource == GameSource.STEAM))
val container = ContainerUtils.getContainer(context, appId)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| suspend fun replaceSteamclientDll(context: Context, appId: String) { | |
| val steamAppId = ContainerUtils.extractGameIdFromContainerId(appId) | |
| val appDirPath = SteamService.getAppDirPath(steamAppId) | |
| val appDirPath = SteamService.getAppDirPath(steamAppId, (appId.contains("${GameSource.STEAM.name}"))) | |
| val container = ContainerUtils.getContainer(context, appId) | |
| suspend fun replaceSteamclientDll(context: Context, appId: String) { | |
| val steamAppId = ContainerUtils.extractGameIdFromContainerId(appId) | |
| val gameSource = ContainerUtils.extractGameSourceFromContainerId(appId) | |
| val appDirPath = SteamService.getAppDirPath(steamAppId, (gameSource == GameSource.STEAM)) | |
| val container = ContainerUtils.getContainer(context, appId) |
🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/utils/SteamUtils.kt` around lines 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.
There was a problem hiding this comment.
I suppose I should take this recommendation, can see potential bugs with epic and gog games that the name "Steam"
There was a problem hiding this comment.
@jks211, great catch! Yes, using extractGameSourceFromContainerId() will properly handle edge cases where game names or IDs might contain the string "STEAM" for non-Steam sources like Epic or GOG. The prefix-matching approach in that helper is more robust and prevents false positives.
🧠 Learnings used
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 344
File: app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:415-419
Timestamp: 2025-12-17T05:14:05.133Z
Learning: In SteamAutoCloud.kt, when uploading files to Steam Cloud API (beginFileUpload and commitFileUpload), the filename parameter intentionally uses different formats: `file.path + file.filename` (relative path without placeholder) when `appInfo.ufs.saveFilePatterns.isEmpty()` is true (fallback case), and `file.prefixPath` (includes placeholder like %SteamUserData%) when patterns are configured. This difference is by design.
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 176
File: app/src/main/java/app/gamenative/utils/SteamUtils.kt:651-655
Timestamp: 2025-09-28T13:54:16.048Z
Learning: In the GameNative project, when creating depots.txt files in SteamUtils.ensureSteamSettings, only create the file when SteamService.getInstalledDepotsOf() returns actual data (non-null). Don't create empty files or write empty strings when no data is available.
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/service/DownloadService.kt:6-6
Timestamp: 2025-09-19T17:07:27.941Z
Learning: In app/src/main/java/app/gamenative/service/DownloadService.kt, the getSizeFromStoreDisplay() function uses runBlocking to call SteamService.getDownloadableDepots() because the latter was converted from synchronous to suspend in this PR. The user confirmed the previous synchronous implementation worked fine, and if performance is acceptable, keeping runBlocking is a valid approach.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
There is a ContainerUtil that has "ExtractGameSourceFromAppId" for this
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
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?
3 participants
…lient reference for non-steam apps
Summary by cubic
Fixes app directory selection for steamclient DLL by detecting Steam vs non-Steam apps. Non-Steam apps now use the internal path; Steam apps use external storage only when enabled.
Written for commit 3691ee8. Summary will update on new commits.
Summary by CodeRabbit