Fix: typing space in Env Variables would lead to crash. + added variable presets for Mango#1208
Fix: typing space in Env Variables would lead to crash. + added variable presets for Mango#1208Catpotatos wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
feat: Added Mango Hud variables to saved presets for fps and frame limiting. removed variable that was not working for device spoofing. Re-ordered WINEDLLOVERRIDES to top.
Catpotatos
changed the title
📝 WalkthroughWalkthroughThe PR updates environment variable handling by adding whitespace trimming to user inputs across UI components, repositioning and modifying environment variable definitions (removing GPU device filtering, adding Mangohud toggle and config), and improving parsing validation to reject malformed entries with empty names. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
There was a problem hiding this comment.
Pull request overview
Fixes crashes when users enter spaces while editing environment variables, and expands preset/suggestion support for common env vars (including MangoHud), improving the env-var editing UX in the app.
Changes:
- Harden
EnvVars.putAll()against empty/invalid tokens when splitting persisted env-var strings. - Add/update known env-var presets: introduce
MANGOHUD/MANGOHUD_CONFIG, moveWINEDLLOVERRIDEShigher, removeDXVK_FILTER_DEVICE_NAME. - Trim env-var name/value inputs in the settings list and creation dialog to avoid leading/trailing whitespace issues.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/main/java/com/winlator/core/envvars/EnvVars.java | Skips empty/invalid tokens when parsing env var strings to prevent crashes. |
| app/src/main/java/com/winlator/core/envvars/EnvVarInfo.kt | Updates known env var definitions: adds MangoHud vars, adjusts WINEDLLOVERRIDES, removes GPU spoofing var preset. |
| app/src/main/java/app/gamenative/ui/component/settings/SettingsEnvVars.kt | Trims edited env-var values in the settings list UI. |
| app/src/main/java/app/gamenative/ui/component/dialog/EnvironmentTab.kt | Trims env-var name/value in the “create env var” dialog. |
Comments suppressed due to low confidence (1)
app/src/main/java/com/winlator/core/envvars/EnvVars.java:30
- This change fixes a crash case in
putAll, but there’s no unit test coverage for parsing env var strings (e.g., multiple consecutive spaces, leading/trailing spaces, or invalid tokens without=). Adding tests would help prevent regressions sinceEnvVars(String)is used to deserialize persisted config values.
public void putAll(String values) {
if (values == null || values.isEmpty()) return;
String[] parts = values.split(" ");
for (String part : parts) {
int index = part.indexOf("=");
if (part.isEmpty() || index < 1) continue;
String name = part.substring(0, index);
String value = part.substring(index+1);
data.put(name, value);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String[] parts = values.split(" "); | ||
| for (String part : parts) { | ||
| int index = part.indexOf("="); | ||
| if (part.isEmpty() || index < 1) continue; | ||
| String name = part.substring(0, index); | ||
| String value = part.substring(index+1); | ||
| data.put(name, value); |
There was a problem hiding this comment.
putAll still uses values.split(" "), so any env var value containing an unescaped space (e.g. DXVK_HUD=fps, frametimes) will be split into multiple tokens and silently truncated/ignored. Consider either (a) storing env vars using toEscapedString() and teaching putAll to split on unescaped spaces + unescape \ , or (b) validating/rejecting spaces in values at input time with a user-facing error, so values aren’t corrupted.
There was a problem hiding this comment.
tested, keyboard doesn't allow spaces to be typed. video shared.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/winlator/core/envvars/EnvVars.java (1)
21-29:⚠️ Potential issue | 🔴 CriticalThis still breaks env var values that contain spaces.
Skipping malformed tokens avoids the crash, but callers still persist a space-delimited flat string (
EnvironmentTab.ktLine 206 usesenvVars.toString()). A value likeFOO=hello worldnow round-trips here asFOO=helloplus a skippedworldtoken, so the crash becomes silent config corruption instead of a real fix. Please switch to a serializer/parser pair that preserves spaces consistently, e.g. escaped serialization plus matching unescape logic here, or a structured format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/core/envvars/EnvVars.java` around lines 21 - 29, The current putAll method in EnvVars.java corrupts values containing spaces because it splits on plain spaces; update the parser to match a safe serializer used by EnvironmentTab.kt (do not split on spaces inside quoted values or escaped spaces) or switch to a structured format (e.g., JSON, Properties, or newline-separated key=value) and parse that instead; specifically, change putAll(String values) so it decodes the same escaped/quoted format that the corresponding envVars.toString() produces and then call data.put(name, value) with the correctly unescaped full value for each parsed token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/java/com/winlator/core/envvars/EnvVars.java`:
- Around line 21-29: The current putAll method in EnvVars.java corrupts values
containing spaces because it splits on plain spaces; update the parser to match
a safe serializer used by EnvironmentTab.kt (do not split on spaces inside
quoted values or escaped spaces) or switch to a structured format (e.g., JSON,
Properties, or newline-separated key=value) and parse that instead;
specifically, change putAll(String values) so it decodes the same escaped/quoted
format that the corresponding envVars.toString() produces and then call
data.put(name, value) with the correctly unescaped full value for each parsed
token.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec396b53-fda9-4786-9876-e91197e54ad5
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/ui/component/dialog/EnvironmentTab.ktapp/src/main/java/app/gamenative/ui/component/settings/SettingsEnvVars.ktapp/src/main/java/com/winlator/core/envvars/EnvVarInfo.ktapp/src/main/java/com/winlator/core/envvars/EnvVars.java
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 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/ui/component/dialog/EnvironmentTab.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/EnvironmentTab.kt:163">
P2: Value field trims on every keystroke, causing incorrect/unnatural entry of spaced env var values.</violation>
</file>
<file name="app/src/main/java/com/winlator/core/envvars/EnvVars.java">
<violation number="1" location="app/src/main/java/com/winlator/core/envvars/EnvVars.java:26">
P2: The new guard prevents crashes on space-containing values but introduces silent data corruption: `putAll` splits on `" "`, so a value like `DXVK_HUD=fps, frametimes` becomes tokens `DXVK_HUD=fps,` and `frametimes`. The second token has no `=` and is now silently skipped, truncating the stored value. The `.trim()` calls in the UI prevent users from *typing* spaces, but `putAll` can still receive space-containing strings from previously-persisted data or programmatic callers. Consider using an escape-aware delimiter (e.g., storing/splitting on a character that doesn't appear in values, or escaping spaces) so round-tripping is lossless.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| NoExtractOutlinedTextField( | ||
| value = envVarValue, | ||
| onValueChange = { envVarValue = it }, | ||
| onValueChange = { envVarValue = it.trim() }, |
There was a problem hiding this comment.
P2: Value field trims on every keystroke, causing incorrect/unnatural entry of spaced env var values.
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/ui/component/dialog/EnvironmentTab.kt, line 163:
<comment>Value field trims on every keystroke, causing incorrect/unnatural entry of spaced env var values.</comment>
<file context>
@@ -160,7 +160,7 @@ fun EnvironmentTabContent(state: ContainerConfigState) {
NoExtractOutlinedTextField(
value = envVarValue,
- onValueChange = { envVarValue = it },
+ onValueChange = { envVarValue = it.trim() },
label = { Text(text = stringResource(R.string.value)) },
singleLine = true,
</file context>
| onValueChange = { envVarValue = it.trim() }, | |
| onValueChange = { envVarValue = it }, |
| String[] parts = values.split(" "); | ||
| for (String part : parts) { | ||
| int index = part.indexOf("="); | ||
| if (part.isEmpty() || index < 1) continue; |
There was a problem hiding this comment.
P2: The new guard prevents crashes on space-containing values but introduces silent data corruption: putAll splits on " ", so a value like DXVK_HUD=fps, frametimes becomes tokens DXVK_HUD=fps, and frametimes. The second token has no = and is now silently skipped, truncating the stored value. The .trim() calls in the UI prevent users from typing spaces, but putAll can still receive space-containing strings from previously-persisted data or programmatic callers. Consider using an escape-aware delimiter (e.g., storing/splitting on a character that doesn't appear in values, or escaping spaces) so round-tripping is lossless.
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/core/envvars/EnvVars.java, line 26:
<comment>The new guard prevents crashes on space-containing values but introduces silent data corruption: `putAll` splits on `" "`, so a value like `DXVK_HUD=fps, frametimes` becomes tokens `DXVK_HUD=fps,` and `frametimes`. The second token has no `=` and is now silently skipped, truncating the stored value. The `.trim()` calls in the UI prevent users from *typing* spaces, but `putAll` can still receive space-containing strings from previously-persisted data or programmatic callers. Consider using an escape-aware delimiter (e.g., storing/splitting on a character that doesn't appear in values, or escaping spaces) so round-tripping is lossless.</comment>
<file context>
@@ -23,6 +23,7 @@ public void putAll(String values) {
String[] parts = values.split(" ");
for (String part : parts) {
int index = part.indexOf("=");
+ if (part.isEmpty() || index < 1) continue;
String name = part.substring(0, index);
String value = part.substring(index+1);
</file context>
There was a problem hiding this comment.
keyboard will not allow to type spaces. several barriers in place to ensure a space, that could break the environment variable intention, won't go through.
Values should not contain spaces,
so typing "DXVK_HUD=fps, frametimes" should actually always become DXVK_HUD=fps,frametimes and parse correctly as intended.
Before this implementation, entering a space would mean a crash. now it blocks it from happening.
|
not sure the mango changes are needed, #1081 is doing fps cap in a different way |
Description
Fix: typing space in Env Variables would lead to crash.
Recording
screen-20260413-214813.1.mp4
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
Fixes a crash when typing spaces in Environment Variables and adds MangoHud presets for FPS display and limiting. Also removes a broken spoofing preset and moves
WINEDLLOVERRIDESto the top for quicker access.Bug Fixes
EnvVars.putAllto avoid crashes from stray spaces.New Features
MANGOHUD(toggle) andMANGOHUD_CONFIGpresets (e.g., fps, fps_limit=60, fps_only).WINEDLLOVERRIDESsuggestions and place it first in the list.DXVK_FILTER_DEVICE_NAMEpreset.Written for commit 05fe001. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features