Skip to content

fix: default FPS limiter to disabled for pre-0.9.1 containers#1390

Open
xXJSONDeruloXx wants to merge 1 commit into
utkarshdalal:masterfrom
xXJSONDeruloXx:fix/fps-limiter-default-enabled
Open

fix: default FPS limiter to disabled for pre-0.9.1 containers#1390
xXJSONDeruloXx wants to merge 1 commit into
utkarshdalal:masterfrom
xXJSONDeruloXx:fix/fps-limiter-default-enabled

Conversation

@xXJSONDeruloXx
Copy link
Copy Markdown
Contributor

@xXJSONDeruloXx xXJSONDeruloXx commented May 7, 2026

Description

The built-in FPS limiter introduced in 0.9.1 defaults to enabled at 60fps. For every container created before 0.9.1 (which is essentially all of them), the limiter silently activates on first launch because there is no saved fpsLimiterEnabled extra yet. The parseBooleanExtra call falls through to the hardcoded true default.

This causes users to be capped at 60fps regardless of disabling the sidebar toggle, removing DXVK_FRAME_RATE from environment, or setting in-game fps to unlimited. The only workaround is manually toggling the limiter on then off in the quick menu so the false value gets persisted, or reverting to 0.9.0.

The fix is a one-character change: default initialFpsLimiterEnabled to false instead of true. Existing containers where the user already toggled the limiter on are unaffected since their saved extra takes precedence. New containers and pre-0.9.1 containers will now start uncapped, and users can opt in to the limiter from the quick menu.

Discord reports

https://discord.com/channels/1378308569287622737/1501131331575218307
https://discord.com/channels/1378308569287622737/1501640621704609812
https://discord.com/channels/1378308569287622737/1500055122766463026

Recording

Type of Change

  • Bug fix
  • Performance / stability improvement
  • Compatibility improvements
  • Other (requires prior approval)

Checklist

  • If I have access to #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.
  • This change aligns with the current project scope (core functionality, stability, or performance). If not, it has been explicitly approved beforehand.
  • I have attached a recording of the change.
  • I have read and agree to the contribution guidelines in CONTRIBUTING.md.

Summary by cubic

Default the built-in FPS limiter to off for containers created before 0.9.1 to prevent a silent 60 FPS cap on first launch.

  • Bug Fixes
    • Changed fallback for fpsLimiterEnabled to false when the extra is missing.
    • Containers with a saved setting are unaffected; new and legacy containers start uncapped and can enable the limiter from the quick menu.

Written for commit 0178b80. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Updated FPS limiter initialization to default to disabled when configuration is unavailable, improving initial application performance and startup experience.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR changes the default FPS limiter behavior in XServerScreen.kt. When a container lacks the fpsLimiterEnabled extra attribute, the function now defaults to false instead of true. The surrounding FPS limiter target initialization remains unaffected.

Changes

FPS Limiter Default Initialization

Layer / File(s) Summary
Default FPS Limiter Initialization
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
The initialFpsLimiterEnabled(container: Container) function default value changes from true to false when the fpsLimiterEnabled container extra is missing.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • utkarshdalal

Poem

A rabbit hops through FPS frames so fast,
Default now set to false at last,
The limiter rests with a gentle false command,
Smoother gameplay across the land! 🐰⚡

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: defaulting the FPS limiter to disabled for pre-0.9.1 containers, which directly addresses the bug being fixed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is thorough and well-structured. It explains the problem, the impact, the fix, and provides context with Discord report links.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xXJSONDeruloXx xXJSONDeruloXx marked this pull request as ready for review May 7, 2026 14:49
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@utkarshdalal
Copy link
Copy Markdown
Owner

Isn't the fix here that the toggle should show as enabled rather than changing the default?

@utkarshdalal
Copy link
Copy Markdown
Owner

@xXJSONDeruloXx please see comment above.

@xXJSONDeruloXx
Copy link
Copy Markdown
Contributor Author

No, we should let users opt in, and if true and they toggle on fg they're locked into fps cap when ui disagrees with no way to affect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants