✨ [feat] [draft] Fullscreen stretch feature#558
Open
Faustasm wants to merge 3 commits intoutkarshdalal:masterfrom
Open
✨ [feat] [draft] Fullscreen stretch feature#558Faustasm wants to merge 3 commits intoutkarshdalal:masterfrom
Faustasm wants to merge 3 commits intoutkarshdalal:masterfrom
Conversation
Faustasm
changed the title
Contributor
📝 WalkthroughWalkthroughAdds a new boolean stretchToFullscreen preference and container flag; exposes a UI toggle; persists and propagates the flag through ContainerData/Container, Intent parsing/merge, container utilities, tests, resources, and triggers a renderer fullscreen toggle on container launch when enabled. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as GeneralTab
participant Pref as PrefManager
participant Util as ContainerUtils
participant Container as Container
participant XServer as XServerScreen
participant Renderer as Renderer
User->>UI: Toggle "Stretch to fullscreen"
UI->>Pref: persist stretchToFullscreen
UI->>Util: update ContainerData with stretchToFullscreen
Util->>Container: applyToContainer(stretchToFullscreen)
Container->>Container: saveData() (store stretchToFullscreen)
User->>XServer: Launch container
XServer->>Container: isStretchToFullscreen()
Container-->>XServer: true
alt stretch enabled
XServer->>Renderer: toggleFullscreen()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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}
Contributor
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (all 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/ui/component/dialog/GeneralTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt:307">
P3: Hardcoded UI string literal prevents localization; use a string resource instead.</violation>
</file>
<file name="app/src/main/java/com/winlator/container/ContainerData.kt">
<violation number="1" location="app/src/main/java/com/winlator/container/ContainerData.kt:167">
P2: Unsafe cast on newly added `stretchToFullscreen` will crash when restoring legacy saved maps that lack this key; use a safe cast with a default fallback.</violation>
</file>
<file name="app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt:286">
P2: `stretchToFullscreen` overrides cannot disable a `true` base value because the merge treats `false` as “no override.” Explicit `false` in the override is ignored, so users cannot turn the feature off via intent overrides.</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
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - 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.
| executablePath = savedMap["executablePath"] as String, | ||
| installPath = savedMap["installPath"] as String, | ||
| showFPS = savedMap["showFPS"] as Boolean, | ||
| stretchToFullscreen = savedMap["stretchToFullscreen"] as Boolean, |
Contributor
There was a problem hiding this comment.
P2: Unsafe cast on newly added stretchToFullscreen will crash when restoring legacy saved maps that lack this key; use a safe cast with a default fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/container/ContainerData.kt, line 167:
<comment>Unsafe cast on newly added `stretchToFullscreen` will crash when restoring legacy saved maps that lack this key; use a safe cast with a default fallback.</comment>
<file context>
@@ -162,6 +164,7 @@ data class ContainerData(
executablePath = savedMap["executablePath"] as String,
installPath = savedMap["installPath"] as String,
showFPS = savedMap["showFPS"] as Boolean,
+ stretchToFullscreen = savedMap["stretchToFullscreen"] as Boolean,
launchRealSteam = savedMap["launchRealSteam"] as Boolean,
allowSteamUpdates = savedMap["allowSteamUpdates"] as Boolean,
</file context>
Suggested change
| stretchToFullscreen = savedMap["stretchToFullscreen"] as Boolean, | |
| stretchToFullscreen = (savedMap["stretchToFullscreen"] as? Boolean) ?: false, |
| installPath = override.installPath.ifEmpty { base.installPath }, | ||
| // Boolean fields: only override if different from parsing defaults | ||
| showFPS = if (override.showFPS != false) override.showFPS else base.showFPS, | ||
| stretchToFullscreen = if (override.stretchToFullscreen != false) override.stretchToFullscreen else base.stretchToFullscreen, |
Contributor
There was a problem hiding this comment.
P2: stretchToFullscreen overrides cannot disable a true base value because the merge treats false as “no override.” Explicit false in the override is ignored, so users cannot turn the feature off via intent overrides.
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/utils/IntentLaunchManager.kt, line 286:
<comment>`stretchToFullscreen` overrides cannot disable a `true` base value because the merge treats `false` as “no override.” Explicit `false` in the override is ignored, so users cannot turn the feature off via intent overrides.</comment>
<file context>
@@ -282,6 +283,7 @@ object IntentLaunchManager {
installPath = override.installPath.ifEmpty { base.installPath },
// Boolean fields: only override if different from parsing defaults
showFPS = if (override.showFPS != false) override.showFPS else base.showFPS,
+ stretchToFullscreen = if (override.stretchToFullscreen != false) override.stretchToFullscreen else base.stretchToFullscreen,
launchRealSteam = if (override.launchRealSteam != false) override.launchRealSteam else base.launchRealSteam,
cpuList = if (override.cpuList != Container.getFallbackCPUList()) override.cpuList else base.cpuList,
</file context>
app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
Outdated
Show resolved
Hide resolved
![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: 1
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt (1)
286-286: Optional: allow explicit “false” overrides.Current
!= falselogic treatsfalseas “unset,” so overrides can’t disable an existingtrue. If external launchers need an explicit “off,” consider a nullable flag or presence-aware parsing for overrides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt` at line 286, The current assignment for stretchToFullscreen uses "if (override.stretchToFullscreen != false) ..." which treats explicit false as unset; change it to honor explicit false by using a null-aware selection (e.g. use override.stretchToFullscreen ?: base.stretchToFullscreen) or otherwise check presence instead of "!= false"; update the override type/parsing so override.stretchToFullscreen is nullable Boolean and absent values remain null, then set stretchToFullscreen in IntentLaunchManager to prefer override when non-null else fall back to base.
🤖 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/ui/component/dialog/GeneralTab.kt`:
- Around line 305-310: The label "Stretch to fullscreen" in GeneralTab.kt is
hardcoded; add a string resource (e.g., name="stretch_to_fullscreen") in
strings.xml with the appropriate casing and translations, then update the
SettingsSwitch title lambda in GeneralTab.kt to use
stringResource(R.string.stretch_to_fullscreen) instead of the hardcoded Text so
config.stretchToFullscreen and the existing onCheckedChange remain unchanged.
Ensure the new string key follows existing naming conventions and is included in
translation files as needed.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt`:
- Line 286: The current assignment for stretchToFullscreen uses "if
(override.stretchToFullscreen != false) ..." which treats explicit false as
unset; change it to honor explicit false by using a null-aware selection (e.g.
use override.stretchToFullscreen ?: base.stretchToFullscreen) or otherwise check
presence instead of "!= false"; update the override type/parsing so
override.stretchToFullscreen is nullable Boolean and absent values remain null,
then set stretchToFullscreen in IntentLaunchManager to prefer override when
non-null else fall back to base.
Owner
|
Hey, thanks for the PR, but this is not something we want to implement. We'd like the games to be played in the way the devs intended, in the same aspect ratios. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt (2)
323-332:⚠️ Potential issue | 🟡 MinorDuplicate
wineVersionkey in JSON literalThe JSON string contains
"wineVersion"twice — first"invalid-wine-version"(dead), then"wine-9.2-x86_64"(effective). Duplicate keys are undefined by the JSON spec; the test works becausekotlinx.serializationuses the last occurrence, but this is confusing and looks like a leftover from a previous iteration.🔧 Proposed fix
val glibcConfigJson = """ { - "wineVersion": "invalid-wine-version", - "dxwrapper": "dxvk", + "dxwrapper": "dxvk", "dxwrapperConfig": "version=1.10.3", "containerVariant": "glibc", "box64Version": "0.3.6", "wineVersion": "wine-9.2-x86_64" } """.trimIndent()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt` around lines 323 - 332, The test JSON in BestConfigServiceTest.kt has a duplicate "wineVersion" key in the glibcConfigJson string (first "invalid-wine-version" then "wine-9.2-x86_64"); remove the obsolete/duplicated entry so there is a single "wineVersion" field (keep the intended value "wine-9.2-x86_64") in the glibcConfigJson literal to avoid ambiguity and match the expected test data.
717-717:⚠️ Potential issue | 🟡 MinorContradictory assertion — test will fail at this line
The assertion message says
"Hades2 Adreno835 family result should be empty"but the predicate isisNotEmpty() == true(asserts non-empty).hades2Adreno835FamilyMatchResponse(line 57) has nowineVersionfield; pertestMissingWineVersion_returnsNull, that returnsnull/empty — makingisNotEmpty() == trueevaluate tofalseand causingassertTrueto fail.The inline comment
// Missing wineVersionconfirms the intent is to assert empty. The fix:🐛 Proposed fix
- assertTrue("Hades2 Adreno835 family result should be empty", hades2Adreno835Result?.isNotEmpty() == true) // Missing wineVersion + assertTrue("Hades2 Adreno835 family result should be empty (missing wineVersion)", hades2Adreno835Result == null || hades2Adreno835Result.isEmpty())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt` at line 717, The assertion message and predicate are contradictory: in BestConfigServiceTest (look for hades2Adreno835Result and the testMissingWineVersion_returnsNull test) the intent is to assert the result is empty because hades2Adreno835FamilyMatchResponse is missing wineVersion; update the assertion to check emptiness (e.g., assert that hades2Adreno835Result?.isEmpty() == true or equivalent) and keep the message "Hades2 Adreno835 family result should be empty" so the test correctly expects an empty result when wineVersion is missing.
🤖 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/res/values/strings.xml`:
- Line 488: The string resource "stretch_to_fullscreen" uses Title Case with an
incorrectly capitalized preposition; update its value to sentence case to match
Material Design 3 and adjacent labels—replace "Stretch To Fullscreen" with
"Stretch to fullscreen" (and scan nearby keys like "toggle_fullscreen" /
"hide_on_screen_controls_with_controller" for consistency if needed).
In `@app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt`:
- Line 29: The parser omits the stretchToFullscreen boolean so server values are
ignored; in BestConfigService.parseConfigToContainerData() add extraction of the
"stretchToFullscreen" JSON field (same pattern as existing showFPS extraction)
and map it into the ContainerData.stretchToFullscreen property so
IntentLaunchManager/ContainerUtils see the value, then update the test payloads
in BestConfigServiceTest (replace incorrect "stretchToFullScreen" with
"stretchToFullscreen") and add assertions in BestConfigServiceTest verifying
parsed ContainerData.stretchToFullscreen is correct.
---
Outside diff comments:
In `@app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt`:
- Around line 323-332: The test JSON in BestConfigServiceTest.kt has a duplicate
"wineVersion" key in the glibcConfigJson string (first "invalid-wine-version"
then "wine-9.2-x86_64"); remove the obsolete/duplicated entry so there is a
single "wineVersion" field (keep the intended value "wine-9.2-x86_64") in the
glibcConfigJson literal to avoid ambiguity and match the expected test data.
- Line 717: The assertion message and predicate are contradictory: in
BestConfigServiceTest (look for hades2Adreno835Result and the
testMissingWineVersion_returnsNull test) the intent is to assert the result is
empty because hades2Adreno835FamilyMatchResponse is missing wineVersion; update
the assertion to check emptiness (e.g., assert that
hades2Adreno835Result?.isEmpty() == true or equivalent) and keep the message
"Hades2 Adreno835 family result should be empty" so the test correctly expects
an empty result when wineVersion is missing.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt`:
- Around line 305-310: No change required: the SettingsSwitch in GeneralTab now
correctly uses stringResource(R.string.stretch_to_fullscreen) for its title and
follows the established binding pattern (state = config.stretchToFullscreen and
onCheckedChange = { state.config.value = config.copy(stretchToFullscreen = it)
}) matching the neighboring showFPS switch; keep SettingsSwitch and the
config.copy usage as-is and remove any duplicate review comments.
| <string name="height">Height</string> | ||
| <string name="audio_driver">Audio Driver</string> | ||
| <string name="show_fps">Show FPS</string> | ||
| <string name="stretch_to_fullscreen">Stretch To Fullscreen</string> |
Contributor
There was a problem hiding this comment.
Capitalization of "To" is non-standard — already flagged as a pending draft item.
"Stretch To Fullscreen" capitalizes the preposition "To". Standard English title case lowers short prepositions, and the Android/Material Design 3 convention for settings labels is sentence case. The adjacent section uses both styles inconsistently (e.g., "Toggle Fullscreen" vs "Hide on-screen controls with controller"), so whichever convention is settled on should be applied here too.
✏️ Suggested correction (sentence case, consistent with Material Design)
- <string name="stretch_to_fullscreen">Stretch To Fullscreen</string>
+ <string name="stretch_to_fullscreen">Stretch to fullscreen</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/values/strings.xml` at line 488, The string resource
"stretch_to_fullscreen" uses Title Case with an incorrectly capitalized
preposition; update its value to sentence case to match Material Design 3 and
adjacent labels—replace "Stretch To Fullscreen" with "Stretch to fullscreen"
(and scan nearby keys like "toggle_fullscreen" /
"hide_on_screen_controls_with_controller" for consistency if needed).
| // Sample API responses from the user | ||
| private val cs2Adreno735Response = """ | ||
| {"bestConfig":{"id":"STEAM_730","name":"","drives":"D:/storage/emulated/0/DownloadE:/data/data/app.gamenative/storageA:/data/user/0/app.gamenative/Steam/steamapps/common/Counter-Strike Global Offensive","lc_all":"en_US.utf8","cpuList":"0,1,2,3,4,5,6,7","envVars":"ZINK_DESCRIPTORS=lazy ZINK_DEBUG=compact MESA_SHADER_CACHE_DISABLE=false MESA_SHADER_CACHE_MAX_SIZE=512MB mesa_glthread=true WINEESYNC=1 MESA_VK_WSI_PRESENT_MODE=mailbox TU_DEBUG=noconform DXVK_FRAME_RATE=60","showFPS":true,"useDRI3":false,"emulator":"Box64","execArgs":"","forceDlc":false,"language":"english","rcfileId":0,"dxwrapper":"dxvk","extraData":{"dxwrapper":"dxvk-async-1.10.3","appVersion":"6","imgVersion":"24","audioDriver":"pulseaudio","box64Version":"0.3.6","desktopTheme":"LIGHT,IMAGE,#0277bd,640x480","wincomponents":"direct3d=1,directsound=1,directmusic=0,directshow=0,directplay=0,vcrun2010=1,wmdecoder=1,opengl=0","config_changed":"false","fexcoreVersion":"2507","startupSelection":"1","lastInstalledMainWrapper":"wrapper-leegao","discord_support_prompt_shown":"true"},"inputType":3,"steamType":"normal","wow64Mode":true,"screenSize":"640x480","audioDriver":"pulseaudio","box64Preset":"PERFORMANCE","box86Preset":"COMPATIBILITY","installPath":"","wineVersion":"proton-9.0-x86_64","box64Version":"0.3.6","box86Version":"0.3.2","cpuListWoW64":"0,1,2,3,4,5,6,7","desktopTheme":"LIGHT,IMAGE,#0277bd","useLegacyDRM":false,"midiSoundFont":"","wincomponents":"direct3d=1,directsound=1,directmusic=0,directshow=0,directplay=0,vcrun2010=1,wmdecoder=1,opengl=0","executablePath":"game/bin/win64/cs2.exe","fexcoreVersion":"2507","graphicsDriver":"wrapper-leegao","needsUnpacking":false,"dxwrapperConfig":"version=async-1.10.3,framerate=0,maxDeviceMemory=0,async=1,asyncCache=0,vkd3dVersion=2.6,vkd3dLevel=12_1","launchRealSteam":true,"sessionMetadata":{"avg_fps":113.9,"session_length_sec":208},"touchscreenMode":false,"containerVariant":"bionic","dinputMapperType":1,"sdlControllerAPI":true,"startupSelection":1,"allowSteamUpdates":true,"controllerMapping":"","disableMouseInput":false,"primaryController":1,"emulateKeyboardMouse":false,"graphicsDriverConfig":"version=System;blacklistedExtensions=;maxDeviceMemory=0;adrenotoolsTurnip=1;frameSync=Normal","graphicsDriverVersion":"","controllerEmulationBindings":{"A":"KEY_SPACE","B":"KEY_E","X":"KEY_Q","Y":"KEY_TAB","L1":"KEY_SHIFT_L","L2":"MOUSE_LEFT_BUTTON","L3":"NONE","R1":"KEY_CTRL_R","R2":"MOUSE_RIGHT_BUTTON","R3":"NONE","START":"KEY_ENTER","SELECT":"KEY_ESC","DPAD_UP":"KEY_UP","DPAD_DOWN":"KEY_DOWN","DPAD_LEFT":"KEY_LEFT","DPAD_RIGHT":"KEY_RIGHT"}},"matchType":"fallback_match","matchedGpu":"Mali-G57 MC2","matchedDeviceId":7929} | ||
| {"bestConfig":{"id":"STEAM_730","name":"","drives":"D:/storage/emulated/0/DownloadE:/data/data/app.gamenative/storageA:/data/user/0/app.gamenative/Steam/steamapps/common/Counter-Strike Global Offensive","lc_all":"en_US.utf8","cpuList":"0,1,2,3,4,5,6,7","envVars":"ZINK_DESCRIPTORS=lazy ZINK_DEBUG=compact MESA_SHADER_CACHE_DISABLE=false MESA_SHADER_CACHE_MAX_SIZE=512MB mesa_glthread=true WINEESYNC=1 MESA_VK_WSI_PRESENT_MODE=mailbox TU_DEBUG=noconform DXVK_FRAME_RATE=60","showFPS":true,"stretchToFullScreen":true,"useDRI3":false,"emulator":"Box64","execArgs":"","forceDlc":false,"language":"english","rcfileId":0,"dxwrapper":"dxvk","extraData":{"dxwrapper":"dxvk-async-1.10.3","appVersion":"6","imgVersion":"24","audioDriver":"pulseaudio","box64Version":"0.3.6","desktopTheme":"LIGHT,IMAGE,#0277bd,640x480","wincomponents":"direct3d=1,directsound=1,directmusic=0,directshow=0,directplay=0,vcrun2010=1,wmdecoder=1,opengl=0","config_changed":"false","fexcoreVersion":"2507","startupSelection":"1","lastInstalledMainWrapper":"wrapper-leegao","discord_support_prompt_shown":"true"},"inputType":3,"steamType":"normal","wow64Mode":true,"screenSize":"640x480","audioDriver":"pulseaudio","box64Preset":"PERFORMANCE","box86Preset":"COMPATIBILITY","installPath":"","wineVersion":"proton-9.0-x86_64","box64Version":"0.3.6","box86Version":"0.3.2","cpuListWoW64":"0,1,2,3,4,5,6,7","desktopTheme":"LIGHT,IMAGE,#0277bd","useLegacyDRM":false,"midiSoundFont":"","wincomponents":"direct3d=1,directsound=1,directmusic=0,directshow=0,directplay=0,vcrun2010=1,wmdecoder=1,opengl=0","executablePath":"game/bin/win64/cs2.exe","fexcoreVersion":"2507","graphicsDriver":"wrapper-leegao","needsUnpacking":false,"dxwrapperConfig":"version=async-1.10.3,framerate=0,maxDeviceMemory=0,async=1,asyncCache=0,vkd3dVersion=2.6,vkd3dLevel=12_1","launchRealSteam":true,"sessionMetadata":{"avg_fps":113.9,"session_length_sec":208},"touchscreenMode":false,"containerVariant":"bionic","dinputMapperType":1,"sdlControllerAPI":true,"startupSelection":1,"allowSteamUpdates":true,"controllerMapping":"","disableMouseInput":false,"primaryController":1,"emulateKeyboardMouse":false,"graphicsDriverConfig":"version=System;blacklistedExtensions=;maxDeviceMemory=0;adrenotoolsTurnip=1;frameSync=Normal","graphicsDriverVersion":"","controllerEmulationBindings":{"A":"KEY_SPACE","B":"KEY_E","X":"KEY_Q","Y":"KEY_TAB","L1":"KEY_SHIFT_L","L2":"MOUSE_LEFT_BUTTON","L3":"NONE","R1":"KEY_CTRL_R","R2":"MOUSE_RIGHT_BUTTON","R3":"NONE","START":"KEY_ENTER","SELECT":"KEY_ESC","DPAD_UP":"KEY_UP","DPAD_DOWN":"KEY_DOWN","DPAD_LEFT":"KEY_LEFT","DPAD_RIGHT":"KEY_RIGHT"}},"matchType":"fallback_match","matchedGpu":"Mali-G57 MC2","matchedDeviceId":7929} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the structure of the test file
wc -l app/src/test/java/app/gamenative/utils/BestConfigServiceTest.ktRepository: utkarshdalal/GameNative
Length of output: 135
🏁 Script executed:
# Read the test file to see the JSON payloads and assertions
cat -n app/src/test/java/app/gamenative/utils/BestConfigServiceTest.ktRepository: utkarshdalal/GameNative
Length of output: 50379
🏁 Script executed:
# Search for the model class definition to check field name casing
rg -n 'stretchToFull[Ss]creen' --type kotlin -A2 -B2 app/src/Repository: utkarshdalal/GameNative
Length of output: 48409
🏁 Script executed:
# Find the BestConfigService parser implementation
fd -e kt -e java | xargs grep -l 'parseConfigToContainerData' | head -5Repository: utkarshdalal/GameNative
Length of output: 326
🏁 Script executed:
# Read the BestConfigService to see how it parses the JSON
cat -n app/src/main/java/app/gamenative/utils/BestConfigService.kt | head -200Repository: utkarshdalal/GameNative
Length of output: 8752
🏁 Script executed:
# Search for the parseConfigToContainerData function specifically
rg -n 'parseConfigToContainerData|stretchToFull' app/src/main/java/app/gamenative/utils/BestConfigService.kt -A5 -B2Repository: utkarshdalal/GameNative
Length of output: 539
🏁 Script executed:
# Get the full parseConfigToContainerData function
sed -n '636,900p' app/src/main/java/app/gamenative/utils/BestConfigService.kt | cat -nRepository: utkarshdalal/GameNative
Length of output: 11245
🏁 Script executed:
# Check if stretchToFullscreen should be handled in the parser
# by seeing how it's used in the production code
rg -n 'stretchToFullscreen' app/src/main/java/app/gamenative/utils/ -B2 -A2 | head -50Repository: utkarshdalal/GameNative
Length of output: 4790
Add stretchToFullscreen field extraction to BestConfigService.parseConfigToContainerData()
The parser completely omits extracting stretchToFullscreen from the config JSON. While the test payloads use the incorrect casing "stretchToFullScreen" (capital S), the larger issue is that the field is absent from the parser entirely (lines 79–154 in BestConfigService.kt). This field is part of the ContainerData model and actively used in IntentLaunchManager and ContainerUtils, so server-provided values will always be ignored.
Add extraction logic for stretchToFullscreen alongside showFPS and other boolean fields. Additionally, update the test JSON payloads to use the correct lowercase casing ("stretchToFullscreen") and add test assertions verifying the field is present in the parsed result.
Also applies to: 33-33, 37-37, 41-41, 45-45, 49-49, 53-53, 57-57, 61-61, 65-65, 1000-1000
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/test/java/app/gamenative/utils/BestConfigServiceTest.kt` at line 29,
The parser omits the stretchToFullscreen boolean so server values are ignored;
in BestConfigService.parseConfigToContainerData() add extraction of the
"stretchToFullscreen" JSON field (same pattern as existing showFPS extraction)
and map it into the ContainerData.stretchToFullscreen property so
IntentLaunchManager/ContainerUtils see the value, then update the test payloads
in BestConfigServiceTest (replace incorrect "stretchToFullScreen" with
"stretchToFullscreen") and add assertions in BestConfigServiceTest verifying
parsed ContainerData.stretchToFullscreen is correct.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/res/values/strings.xml
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
- app/src/main/java/app/gamenative/utils/ContainerUtils.kt
- app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
🤖 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/PrefManager.kt`:
- Around line 286-291: Remove the entire stretch-to-fullscreen preference and
its usages: delete the booleanPreferencesKey named STRETCH_TO_FULLSCREEN and the
stretchToFullscreen property from PrefManager (and revert any related changes
across the PR that reference these symbols), ensuring any UI, settings screen
entries, or logic that read or wrote stretchToFullscreen are reverted to their
previous behavior so games retain their intended aspect ratios.
Comment on lines
+286
to
+291
| private val STRETCH_TO_FULLSCREEN = booleanPreferencesKey("stretch_to_fullscreen") | ||
| var stretchToFullscreen: Boolean | ||
| get() = getPref(STRETCH_TO_FULLSCREEN, false) | ||
| set(value) { | ||
| setPref(STRETCH_TO_FULLSCREEN, value) | ||
| } |
Contributor
There was a problem hiding this comment.
Feature explicitly rejected by the project maintainer — this change should not be merged.
Per the PR comments, the project maintainer (utkarshdalal) has stated this feature is not desired; games should be played in their intended aspect ratios. The entire stretchToFullscreen preference (and all related changes across the PR) should be reverted before any further review is warranted.
The implementation itself is technically fine and consistent with the existing pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/PrefManager.kt` around lines 286 - 291,
Remove the entire stretch-to-fullscreen preference and its usages: delete the
booleanPreferencesKey named STRETCH_TO_FULLSCREEN and the stretchToFullscreen
property from PrefManager (and revert any related changes across the PR that
reference these symbols), ensuring any UI, settings screen entries, or logic
that read or wrote stretchToFullscreen are reverted to their previous behavior
so games retain their intended aspect ratios.
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
2 participants
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.
Fullscreen stretch feature.
Draft until: change string capitalization style, translations, update tests, clean up code if needed.
Flag is off by default, can be accessed in main and individual container settings. Screenshot is in discord.
Summary by cubic
Adds a “Stretch to fullscreen” option that auto-enables fullscreen when a container launches. It’s off by default and saved globally and per container.
New Features
Refactors
Written for commit b6d6394. Summary will update on new commits.
Summary by CodeRabbit