Updated drm handling so every exe does not need to be unpacked #420
Conversation
📝 WalkthroughWalkthroughThis PR updates DRM asset references to a newer version, introduces conditional gating to skip Steamless processing when launching real Steam or using legacy DRM, renames the executable restoration function for clarity, and enhances the restoration logic to handle multiple executables with improved error handling and per-file logging. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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
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}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt">
<violation number="1" location="app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt:656">
P2: Inconsistent assertion message: The comment says 'NOT overwritten' but the assertion message says 'should be overwritten'. Compare with the similar fix at line 770 where the message was correctly updated to 'should not be overwritten'. This makes test failures confusing to debug.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Verify game.exe is NOT overwritten after first replaceSteamClientDll call | ||
| assertEquals("game.exe should be overwritten after replaceSteamClientDll", | ||
| "unpacked exe content", gameExe.readText()) | ||
| "original exe content", gameExe.readText()) |
There was a problem hiding this comment.
P2: Inconsistent assertion message: The comment says 'NOT overwritten' but the assertion message says 'should be overwritten'. Compare with the similar fix at line 770 where the message was correctly updated to 'should not be overwritten'. This makes test failures confusing to debug.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt, line 656:
<comment>Inconsistent assertion message: The comment says 'NOT overwritten' but the assertion message says 'should be overwritten'. Compare with the similar fix at line 770 where the message was correctly updated to 'should not be overwritten'. This makes test failures confusing to debug.</comment>
<file context>
@@ -645,7 +653,7 @@ class SteamUtilsFileSearchTest {
// Verify game.exe is NOT overwritten after first replaceSteamClientDll call
assertEquals("game.exe should be overwritten after replaceSteamClientDll",
- "unpacked exe content", gameExe.readText())
+ "original exe content", gameExe.readText())
// Verify marker was set
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt`:
- Around line 654-657: The assertion message in SteamUtilsFileSearchTest.kt is
incorrect: update the message for the assertEquals that checks
gameExe.readText() (the assertion about "game.exe should be overwritten after
replaceSteamClientDll") to reflect the actual expectation that the file is NOT
overwritten (e.g., say "game.exe should NOT be overwritten after
replaceSteamClientDll" or similar) so the message matches the expected value
"original exe content".
- Around line 927-929: The test comment above the assertEquals in
SteamUtilsFileSearchTest.kt is outdated — it mentions
restoreUnpackedExecutable/unpacked.exe but the assertion now checks that
game.exe contains the original.exe content; update the comment to say the test
verifies that restoring the original executable (game.exe.original.exe)
overwrites game.exe with the original content (matching the asserted "original
exe content") so the comment matches the assertion and references
game.exe.original.exe and the assertEquals check.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/SteamUtils.ktapp/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-17T05:14:05.133Z
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.
Applied to files:
app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/SteamUtils.kt
📚 Learning: 2025-09-28T13:54:16.048Z
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.
Applied to files:
app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/SteamUtils.kt
📚 Learning: 2025-09-19T17:07:27.941Z
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.
Applied to files:
app/src/main/java/app/gamenative/ui/PluviaMain.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: build
🔇 Additional comments (5)
app/src/main/java/app/gamenative/ui/PluviaMain.kt (1)
1094-1101: LGTM — DRM asset update looks consistent.app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
2260-2339: Nice improvement: guarded Steamless flow with per-file error isolation and cleanup.app/src/main/java/app/gamenative/utils/SteamUtils.kt (3)
245-250: DRM tarball version update looks good.The update from
experimental-drm-20260101.tzsttoexperimental-drm-20260116.tzstis consistent with the PR objective. Ensure this asset is available and deployed alongside this change.
252-252: Correct function call for DRM flow.The change from
restoreUnpackedExecutabletorestoreOriginalExecutablealigns with the PR objective. In the DRM handling flow (replaceSteamclientDll), restoring.original.exebackups is the intended behavior, distinct from the.unpacked.exerestoration inreplaceSteamApiat line 217.
337-341: INI configuration for DLL injection folder looks correct.The
DllsToInjectFolder=extra_dllsentry properly configures ColdClient to load additional DLLs from theextra_dllssubdirectory, which is consistent with the cleanup logic at lines 309-312.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| dosDevicesPath.walkTopDown().maxDepth(10) | ||
| .filter { it.isFile && it.name.endsWith(".original.exe", ignoreCase = true) } | ||
| .forEach { file -> | ||
| try { | ||
| val origPath = file.toPath() | ||
| val originalPath = origPath.parent.resolve(origPath.name.removeSuffix(".original.exe")) | ||
| Timber.i("Found ${origPath.name} at ${origPath.absolutePathString()}, restoring...") | ||
|
|
||
| // Delete the current exe if it exists | ||
| if (Files.exists(originalPath)) { | ||
| Files.delete(originalPath) | ||
| } | ||
| // Delete the current exe if it exists | ||
| if (Files.exists(originalPath)) { | ||
| Files.delete(originalPath) | ||
| } | ||
|
|
||
| // Copy the backup back to the original location | ||
| Files.copy(origPath, originalPath) | ||
| // Copy the backup back to the original location | ||
| Files.copy(origPath, originalPath) | ||
|
|
||
| Timber.i("Restored ${originalPath.fileName} from backup") | ||
| restoredCount++ | ||
| } catch (e: IOException) { | ||
| Timber.w(e, "Failed to restore ${file.name} from backup") | ||
| Timber.i("Restored ${originalPath.fileName} from backup") | ||
| restoredCount++ | ||
| } catch (e: IOException) { | ||
| Timber.w(e, "Failed to restore ${file.name} from backup") | ||
| } |
There was a problem hiding this comment.
Case sensitivity mismatch between filter and suffix removal.
The filter uses ignoreCase = true but removeSuffix(".original.exe") is case-sensitive. If a file is named Game.ORIGINAL.EXE, the filter will match it, but removeSuffix won't strip the suffix correctly, leading to an incorrect target path (e.g., attempting to copy to Game.ORIGINAL.EXE instead of Game).
Proposed fix
dosDevicesPath.walkTopDown().maxDepth(10)
.filter { it.isFile && it.name.endsWith(".original.exe", ignoreCase = true) }
.forEach { file ->
try {
val origPath = file.toPath()
- val originalPath = origPath.parent.resolve(origPath.name.removeSuffix(".original.exe"))
+ val baseName = origPath.name.let { name ->
+ val suffixIndex = name.lastIndexOf(".original.exe", ignoreCase = true)
+ if (suffixIndex > 0) name.substring(0, suffixIndex) else name
+ }
+ val originalPath = origPath.parent.resolve(baseName)
Timber.i("Found ${origPath.name} at ${origPath.absolutePathString()}, restoring...")📝 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.
| dosDevicesPath.walkTopDown().maxDepth(10) | |
| .filter { it.isFile && it.name.endsWith(".original.exe", ignoreCase = true) } | |
| .forEach { file -> | |
| try { | |
| val origPath = file.toPath() | |
| val originalPath = origPath.parent.resolve(origPath.name.removeSuffix(".original.exe")) | |
| Timber.i("Found ${origPath.name} at ${origPath.absolutePathString()}, restoring...") | |
| // Delete the current exe if it exists | |
| if (Files.exists(originalPath)) { | |
| Files.delete(originalPath) | |
| } | |
| // Delete the current exe if it exists | |
| if (Files.exists(originalPath)) { | |
| Files.delete(originalPath) | |
| } | |
| // Copy the backup back to the original location | |
| Files.copy(origPath, originalPath) | |
| // Copy the backup back to the original location | |
| Files.copy(origPath, originalPath) | |
| Timber.i("Restored ${originalPath.fileName} from backup") | |
| restoredCount++ | |
| } catch (e: IOException) { | |
| Timber.w(e, "Failed to restore ${file.name} from backup") | |
| Timber.i("Restored ${originalPath.fileName} from backup") | |
| restoredCount++ | |
| } catch (e: IOException) { | |
| Timber.w(e, "Failed to restore ${file.name} from backup") | |
| } | |
| dosDevicesPath.walkTopDown().maxDepth(10) | |
| .filter { it.isFile && it.name.endsWith(".original.exe", ignoreCase = true) } | |
| .forEach { file -> | |
| try { | |
| val origPath = file.toPath() | |
| val baseName = origPath.name.replace(Regex("\\.original\\.exe$", RegexOption.IGNORE_CASE), "") | |
| val originalPath = origPath.parent.resolve(baseName) | |
| Timber.i("Found ${origPath.name} at ${origPath.absolutePathString()}, restoring...") | |
| // Delete the current exe if it exists | |
| if (Files.exists(originalPath)) { | |
| Files.delete(originalPath) | |
| } | |
| // Copy the backup back to the original location | |
| Files.copy(origPath, originalPath) | |
| Timber.i("Restored ${originalPath.fileName} from backup") | |
| restoredCount++ | |
| } catch (e: IOException) { | |
| Timber.w(e, "Failed to restore ${file.name} from backup") | |
| } |
| // Verify game.exe is NOT overwritten after first replaceSteamClientDll call | ||
| assertEquals("game.exe should be overwritten after replaceSteamClientDll", | ||
| "unpacked exe content", gameExe.readText()) | ||
| "original exe content", gameExe.readText()) | ||
|
|
There was a problem hiding this comment.
Fix assertion message to match the new expectation.
The message still says “should be overwritten” while the expected content is the original EXE.
💡 Suggested tweak
- assertEquals("game.exe should be overwritten after replaceSteamClientDll",
+ assertEquals("game.exe should NOT be overwritten after replaceSteamClientDll",
"original exe content", gameExe.readText())📝 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.
| // Verify game.exe is NOT overwritten after first replaceSteamClientDll call | |
| assertEquals("game.exe should be overwritten after replaceSteamClientDll", | |
| "unpacked exe content", gameExe.readText()) | |
| "original exe content", gameExe.readText()) | |
| // Verify game.exe is NOT overwritten after first replaceSteamClientDll call | |
| assertEquals("game.exe should NOT be overwritten after replaceSteamClientDll", | |
| "original exe content", gameExe.readText()) |
🤖 Prompt for AI Agents
In `@app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt` around
lines 654 - 657, The assertion message in SteamUtilsFileSearchTest.kt is
incorrect: update the message for the assertEquals that checks
gameExe.readText() (the assertion about "game.exe should be overwritten after
replaceSteamClientDll") to reflect the actual expectation that the file is NOT
overwritten (e.g., say "game.exe should NOT be overwritten after
replaceSteamClientDll" or similar) so the message matches the expected value
"original exe content".
| // Verify restoreUnpackedExecutable overwrites game.exe with game.exe.unpacked.exe content | ||
| assertEquals("game.exe should be overwritten with game.exe.unpacked.exe content after second replaceSteamClientDll", | ||
| "unpacked exe content", gameExe.readText()) | ||
| assertEquals("game.exe should be overwritten with game.exe.original.exe content after second replaceSteamClientDll", | ||
| "original exe content", gameExe.readText()) |
There was a problem hiding this comment.
Update the test comment to reflect original.exe restoration.
The assertion now expects original.exe content, but the preceding comment still references restoreUnpackedExecutable/unpacked.exe.
💡 Suggested tweak
- // Verify restoreUnpackedExecutable overwrites game.exe with game.exe.unpacked.exe content
+ // Verify restoreOriginalExecutable overwrites game.exe with game.exe.original.exe content🤖 Prompt for AI Agents
In `@app/src/test/java/app/gamenative/utils/SteamUtilsFileSearchTest.kt` around
lines 927 - 929, The test comment above the assertEquals in
SteamUtilsFileSearchTest.kt is outdated — it mentions
restoreUnpackedExecutable/unpacked.exe but the assertion now checks that
game.exe contains the original.exe content; update the comment to say the test
verifies that restoring the original executable (game.exe.original.exe)
overwrites game.exe with the original content (matching the asserted "original
exe content") so the comment matches the assertion and references
game.exe.original.exe and the assertEquals check.
| } | ||
| } | ||
| if (!container.isUseLegacyDRM && !container.isLaunchRealSteam && !SteamService.isFileInstallable(context, "experimental-drm-20260101.tzst")) { | ||
| if (!container.isUseLegacyDRM && !container.isLaunchRealSteam && !SteamService.isFileInstallable(context, "experimental-drm-20260116.tzst")) { |
There was a problem hiding this comment.
Compared the files inside two version, old version have steamclient_extra_x32.dll and steamclient_extra_x64.dll would you want to delete them?
Btw I don't know what is the source of these dlls can't give much comment.
There was a problem hiding this comment.
In the new tzst, those two files don't exist, only the new .dll for stubbing. I tried with one game and it worked ok.
The files come from experimental_steamclient build of gbe-fork
There was a problem hiding this comment.
Is this PR aiming to reduce the number of exe to be processed by Steamless? the changes only make differences to whether to run it, but the number of exes does not change (as filterExecutablesForSteamless is untouched).
I agree to the logic to run it tough, !isLaunchRealSteam and isUseLegacyDRM
There was a problem hiding this comment.
Yes, it aims to reduce the number of exes, and also sometimes Steamless doesn't work - people report errors. This should work in all situations. Just worried it might break.
|
Yes, I deleted those two files. The new file is suggested by anti denuvo
sanctuary to get around steam stub/Steamless.
Utkarsh
…On Fri, 16 Jan, 2026, 20:13 Joshua Tam, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/src/main/java/app/gamenative/ui/PluviaMain.kt
<#420 (comment)>
:
> @@ -1091,13 +1091,13 @@ fun preLaunchApp(
}
}
}
- if (!container.isUseLegacyDRM && !container.isLaunchRealSteam && !SteamService.isFileInstallable(context, "experimental-drm-20260101.tzst")) {
+ if (!container.isUseLegacyDRM && !container.isLaunchRealSteam && !SteamService.isFileInstallable(context, "experimental-drm-20260116.tzst")) {
Compared the files inside two version, old version have
steamclient_extra_x32.dll and steamclient_extra_x64.dll would you want to
delete them?
Btw I don't know what is the source of these dlls can't give much comment.
—
Reply to this email directly, view it on GitHub
<#420 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4SWC6X4R57VGZHHGQXSZT4HD2KRAVCNFSM6AAAAACR43F7P6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNZRGIYDINBYGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Yes, reduce the number of files to unpack, also not all files were being
correctly unpacked. This attempts to make it work for more games...
Utkarsh
…On Fri, 16 Jan, 2026, 20:28 Joshua Tam, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
On app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
<#420 (comment)>
:
Is this PR aiming to reduce the number of exe to be processed by
Steamless? the changes only make differences to whether to run it, but the
number of exes does not change (as filterExecutablesForSteamless is
untouched).
—
Reply to this email directly, view it on GitHub
<#420 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4SWC6L4XJBJLXPSK2TOLD4HD4BRAVCNFSM6AAAAACR43F7P6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMNZRGI3TEOJXGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
| [Injection] | ||
| IgnoreLoaderArchDifference=1 | ||
| DllsToInjectFolder=extra_dlls |
There was a problem hiding this comment.
So this injection is missing before?
What is expected between new version and old version?
There was a problem hiding this comment.
In new version after this PR, without running Steamless, we will be able to launch games because we are loading the extra_dlls which contains a new dll: StubDrm64.dll
My question is, would you want to delete this 2 files inside the app? something like add logic to check existing files and remove those old dll? |
Hm...I use AS to check the diff of |
|
This craters performance (as in startup goes from 60fps to 1 fps before settling at ~28fps) for some games (Divinity Original Sin 1+2) unless Use Legacy DRM is set |
|
@Trixarian dang - in extra dlls, try removing everything except the stub drm dll and let me know if that helps? |
Summary by cubic
Reworked DRM handling so we don't unpack every executable. Steamless now runs only for legacy DRM, and original .exe files are backed up and restored automatically.
Refactors
Dependencies
Written for commit 94086d9. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.