Humongous entertainment/ scumm vm gamefix#1204
Humongous entertainment/ scumm vm gamefix#1204Pepelespooder wants to merge 12 commits intoutkarshdalal:masterfrom
Conversation
Introduce ScummGameFix implementation to detect bundled ScummVM executables and local .ini files, disable update checks, determine the correct game section, and rewrite the container launch command to start the game via ScummVM. Add numerous STEAM_* KeyedGameFix entries for various classic SCUMM titles and update GameFixesRegistry to register them. The ScummGameFix logs actions via Timber and safely handles missing files or parsing errors.
Forgot to remove these in the original commit sorry
Introduce IniFileFix (and KeyedIniFileFix) to centrally update or add key=value entries in INI files (UTF-8), support an optional migrationKey flag, log outcomes and mark container extras. Refactor ScummGameFix to use IniFileFix to disable updates_check and to use LaunchArgFix for launch arguments instead of manual file edits; remove the old disableUpdatesCheckInIni helper. Update GameFixesRegistry by reordering and adding multiple STEAM_Fix entries (new IDs added and some moved) to keep the registry in sync with available fixes.
…inment/-ScummVM-Gamefix
📝 WalkthroughWalkthroughThis PR introduces ScummVM game fix infrastructure by adding 33 new Steam game fix constants for ScummVM-based games, implementing Changes
Sequence Diagram(s)sequenceDiagram
participant Client as GameFix Applier
participant ScummFix as ScummGameFix
participant FileSystem as File System
participant IniParser as INI Parser
participant IniHandler as IniFileFix
participant LaunchHandler as LaunchArgFix
participant Container as Container
Client->>ScummFix: apply(context, gameId, installPath, ...)
ScummFix->>FileSystem: locate scummvm.exe
alt Executable not found
ScummFix-->>Client: return false
else Executable found
ScummFix->>FileSystem: search for scummvm.ini
alt INI not found
ScummFix-->>Client: return false
else INI found
ScummFix->>IniParser: parse scummvm.ini
IniParser-->>ScummFix: detected gameId
ScummFix->>IniHandler: apply IniFileFix (disable updates_check)
IniHandler->>FileSystem: write updated ini
ScummFix->>LaunchHandler: apply LaunchArgFix
LaunchHandler->>Container: set execArgs with ini path + gameId
ScummFix-->>Client: return true
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
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)
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 |
|
Only thing is I dont know if im still over reaching a bit with the Gamefix. I was thinking of just submitting each config individually but than it wouldnt have the softlock fix. Because if update gets set to anything but never it completely breaks game launch on second launch |
Adds CompositeKey , Uses composite key , And added unit tests
|
Im gonna mark this as ready for review. As UnbelievableFlavour has looked it over and helped me fix it up a bit |
unbelievableflavour
left a comment
There was a problem hiding this comment.
Nice PR, lots of nice game fixes + 2 new gamefix types we will surely be using in future PRs as well.
There was a problem hiding this comment.
3 issues found across 38 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt">
<violation number="1" location="app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt:31">
P2: Duplicate fix registrations are silently overwritten when building the map with `associateBy`, leaving dead entries and increasing misconfiguration risk.</violation>
</file>
<file name="app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt">
<violation number="1" location="app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt:51">
P2: ScummGameFix discards CompositeGameFix failure and always returns success, masking sub-fix failures.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt:113">
P2: ScummVM INI selection is order-dependent and may choose an unrelated `.ini`, leading to wrong config mutation or launch failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| GOG_Fix_2147483047, | ||
| GOG_Fix_1787707874, | ||
| GOG_Fix_1635627436, | ||
| STEAM_Fix_400, |
There was a problem hiding this comment.
P2: Duplicate fix registrations are silently overwritten when building the map with associateBy, leaving dead entries and increasing misconfiguration risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt, line 31:
<comment>Duplicate fix registrations are silently overwritten when building the map with `associateBy`, leaving dead entries and increasing misconfiguration risk.</comment>
<file context>
@@ -28,14 +28,49 @@ object GameFixesRegistry {
GOG_Fix_2147483047,
GOG_Fix_1787707874,
GOG_Fix_1635627436,
+ STEAM_Fix_400,
+ STEAM_Fix_1637320,
STEAM_Fix_22300,
</file context>
|
|
||
| private fun findScummVmIni(installPath: String, exeDir: File): File? { | ||
| val candidates = listOfNotNull( | ||
| exeDir.listFiles()?.firstOrNull { it.extension.lowercase() == "ini" }, |
There was a problem hiding this comment.
P2: ScummVM INI selection is order-dependent and may choose an unrelated .ini, leading to wrong config mutation or launch failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt, line 113:
<comment>ScummVM INI selection is order-dependent and may choose an unrelated `.ini`, leading to wrong config mutation or launch failure.</comment>
<file context>
@@ -0,0 +1,131 @@
+
+ private fun findScummVmIni(installPath: String, exeDir: File): File? {
+ val candidates = listOfNotNull(
+ exeDir.listFiles()?.firstOrNull { it.extension.lowercase() == "ini" },
+ File(installPath).listFiles()?.firstOrNull { it.extension.lowercase() == "ini" },
+ )
</file context>
| ), | ||
| LaunchArgFix("-c \"$windowsIniPath\" $detectedGameId"), | ||
| ), | ||
| ).apply(context, gameId, installPath, installPathWindows, container) |
There was a problem hiding this comment.
P2: ScummGameFix discards CompositeGameFix failure and always returns success, masking sub-fix failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt, line 51:
<comment>ScummGameFix discards CompositeGameFix failure and always returns success, masking sub-fix failures.</comment>
<file context>
@@ -0,0 +1,131 @@
+ ),
+ LaunchArgFix("-c \"$windowsIniPath\" $detectedGameId"),
+ ),
+ ).apply(context, gameId, installPath, installPathWindows, container)
+
+ Timber.tag("GameFixes").i("Detected and using local ScummVM config: $windowsIniPath (Game ID: $detectedGameId)")
</file context>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt (1)
106-108: Log the parse exception instead of swallowing it.If reading the INI fails, this collapses I/O or encoding problems into the same "could not detect Game ID" path. Keeping the exception in the log will make bad installs much easier to diagnose.
🪵 Suggested logging tweak
} catch (e: Exception) { + Timber.tag("GameFixes").w(e, "Failed to parse ScummVM ini: ${iniFile.absolutePath}") null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt` around lines 106 - 108, The catch block in ScummGameFix.kt currently swallows exceptions ("catch (e: Exception) { null }"); change it to log the exception before returning null—use the project's logging facility (e.g., the existing logger instance or Android Log.e/Timber) to record e and a short contextual message like "Failed to parse INI while detecting Game ID" so I/O/encoding errors are visible while preserving the null return path.app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt (1)
31-73: Remove the duplicate registrations forSTEAM_Fix_400andSTEAM_Fix_22330.Both keys already appear later in this same list, and
associateBykeeps only the last entry. These extra registrations are dead data today and can hide an intendedKeyedCompositeGameFixif a second fix was meant to stack on the same game key.🧹 Suggested cleanup
- STEAM_Fix_400, STEAM_Fix_1637320, STEAM_Fix_22300, - STEAM_Fix_22330, STEAM_Fix_22380,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt` around lines 31 - 73, In GameFixesRegistry (the list of registered fixes), remove the duplicate enum entries STEAM_Fix_400 and STEAM_Fix_22330 so each key appears only once; locate the repeated occurrences in the collection being built (used later with associateBy) and delete the earlier duplicates so the intended final mapping and any potential KeyedCompositeGameFix stacking are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/gamefixes/STEAM_292240.kt`:
- Around line 5-7: Update the KDoc header in STEAM_292240.kt so the documented
game title matches the actual app ID 292240; locate the top KDoc block (the /**
... */ comment) in the STEAM_292240.kt file and replace the incorrect Spy Fox
title with the correct title for app ID 292240, ensuring the comment accurately
reflects the game's name used elsewhere in mapping/lookup code.
In `@app/src/main/java/app/gamenative/gamefixes/STEAM_292780.kt`:
- Around line 5-7: Update the KDoc title in the top-of-file comment in
STEAM_292780.kt to the correct game name for app ID 292780: replace "Pajama Sam
3: You Are What You Eat From Your Head to Your Feet (Steam)" with the proper
title for Pajama Sam 2 so the file comment accurately reflects app ID 292780;
locate the KDoc block at the top of STEAM_292780.kt and edit the title line
accordingly.
In `@app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt`:
- Around line 111-116: In findScummVmIni, don't pick the first .ini arbitrarily;
explicitly look for a file named "scummvm.ini" in exeDir and in installPath (use
exeDir.resolve("scummvm.ini")/File(installPath,"scummvm.ini") or filter by name
equalsIgnoreCase), returning the first match, and only if neither exists fall
back to the current listFiles().firstOrNull logic; update the function to prefer
case-insensitive exact "scummvm.ini" checks before the generic candidate list.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt`:
- Around line 31-73: In GameFixesRegistry (the list of registered fixes), remove
the duplicate enum entries STEAM_Fix_400 and STEAM_Fix_22330 so each key appears
only once; locate the repeated occurrences in the collection being built (used
later with associateBy) and delete the earlier duplicates so the intended final
mapping and any potential KeyedCompositeGameFix stacking are preserved.
In `@app/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.kt`:
- Around line 106-108: The catch block in ScummGameFix.kt currently swallows
exceptions ("catch (e: Exception) { null }"); change it to log the exception
before returning null—use the project's logging facility (e.g., the existing
logger instance or Android Log.e/Timber) to record e and a short contextual
message like "Failed to parse INI while detecting Game ID" so I/O/encoding
errors are visible while preserving the null return path.
🪄 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: 2e9d4c30-dadf-446d-bb1b-9a3c2ced4548
📒 Files selected for processing (38)
app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283900.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283910.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283920.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283930.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283940.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283960.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_283980.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292240.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292260.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292280.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292770.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292780.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292800.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292820.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_292840.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294530.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294540.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294550.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294560.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294660.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294670.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294680.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294690.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294700.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294710.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_294720.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_317020.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_317030.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_363050.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_363060.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_363070.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_376760.ktapp/src/main/java/app/gamenative/gamefixes/STEAM_376770.ktapp/src/main/java/app/gamenative/gamefixes/types/KeyedCompositeGameFix.ktapp/src/main/java/app/gamenative/gamefixes/types/ScummGameFix.ktapp/src/test/java/app/gamenative/gamefixes/types/KeyedCompositeGameFixTest.ktapp/src/test/java/app/gamenative/gamefixes/types/ScummGameFixTest.kt
Description
Added a ScummGameFix And used inifilefix. Scummgames like freddi fish and others from the humungous series don't run on Game native unless you point it towards the inifile and than the
Game Folder. Also disabled the updater in the Inifile so the user cant soft lock them self by trying to update the scummvm install as it black screens. I have tested a couple others but haven't been able to get them to work why this is only applying to the Humongous series as I've tested all those. The only thing I cant do is set the games as legacy DRM which passes the executable variables properly. Will have to set that as a config option.Opened as Draft PR as im still testing and refining
Recording
https://drive.google.com/file/d/1t1v4xyZHfbYZHQzQu6cIiTpFSPiLOrvI/view?usp=sharing
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
Add a ScummVM game fix for Humongous Entertainment Steam releases. Games now launch via the bundled ScummVM with the right INI, and updates are disabled to avoid black-screen softlocks.
ScummGameFix: detectsscummvm.exe, reads the local.ini, selects the right game section, sets updates_check=0 viaIniFileFix, and rewrites launch args withLaunchArgFix(-c "" ).KeyedCompositeGameFix: new helper to run multiple fixes in order; used byScummGameFix.ScummGameFixandKeyedCompositeGameFix.Written for commit 877da62. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests