feat(game-menu): add dual-screen streaming controls - #451
Open
lhf1a wants to merge 10 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe pull request adds dual-screen display selection, external-display presentation management, a control dashboard, input forwarding, connection metrics, localized resources, and configurable idle backgrounds. ChangesDual-screen streaming
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Game
participant ExternalDisplayManager
participant DualScreenControlPanel
participant ConnectionCallbackHandler
Game->>ExternalDisplayManager: initialize and reconcile displays
ExternalDisplayManager->>Game: onDualScreenControlPanelReady
Game->>DualScreenControlPanel: initialize and show target display
ConnectionCallbackHandler->>DualScreenControlPanel: forward connection stage and quality
Game->>DualScreenControlPanel: update performance metrics
ExternalDisplayManager->>Game: forward control-display input
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
app/src/main/res/drawable/dual_screen_danger_bg.xml (1)
5-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the hardcoded fills into named colors.
dual_screen_action_bg.xmlreferences named colors for both fills. Here the fills are inline hex values while the stroke uses@color/dual_screen_danger. Add the two danger fills toadvance_setting_colors.xmland reference them, so the palette stays in one place.♻️ Proposed refactor
<item android:state_pressed="true"> <shape android:shape="rectangle"> - <solid android:color="`#553D0F16`" /> + <solid android:color="`@color/dual_screen_danger_pressed`" /> <corners android:radius="12dp" /> </shape> </item> <item> <shape android:shape="rectangle"> - <solid android:color="`#33241118`" /> + <solid android:color="`@color/dual_screen_danger_background`" />Add to
app/src/main/res/values/advance_setting_colors.xml:<color name="dual_screen_danger_pressed">`#553D0F16`</color> <color name="dual_screen_danger_background">`#33241118`</color>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/res/drawable/dual_screen_danger_bg.xml` around lines 5 - 11, Replace the inline fill colors in the dual_screen_danger_bg drawable with named color references. Add dual_screen_danger_pressed and dual_screen_danger_background to advance_setting_colors.xml using the existing hex values, then reference those resources for the corresponding shape fills while leaving the stroke unchanged.app/src/main/java/com/limelight/ExternalDisplayManager.kt (1)
227-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeduplicate the identical log message.
Both branches log the same text. Only the following action differs. Log once before the branch.
♻️ Proposed refactor
- if (streamDisplay.displayId != Display.DEFAULT_DISPLAY) { - LimeLog.info( - "Using selected stream display: ${streamDisplay.name} " + - "(ID: ${streamDisplay.displayId}); controls on display ${controlDisplay.displayId}" - ) - startExternalDisplayPresentation(streamDisplay) - return - } - LimeLog.info( "Using selected stream display: ${streamDisplay.name} " + "(ID: ${streamDisplay.displayId}); controls on display ${controlDisplay.displayId}" ) + if (streamDisplay.displayId != Display.DEFAULT_DISPLAY) { + startExternalDisplayPresentation(streamDisplay) + return + } +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/limelight/ExternalDisplayManager.kt` around lines 227 - 239, Deduplicate the repeated selected-stream-display log in the surrounding ExternalDisplayManager flow by moving the identical LimeLog.info call before the streamDisplay.displayId branch. Keep the branch-specific behavior unchanged: startExternalDisplayPresentation(streamDisplay) and return only for non-default displays, while default displays continue through the existing path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/limelight/binding/input/advance_setting/KeyboardUIController.kt`:
- Around line 137-139: Update the three visibility assignments in the keyboard
UI initialization to use nullable-safe lookups for btn_keyboard_resize,
keyboard_resize_handle, and keyboard_opacity_label, preventing dereferences when
optional controls are absent from the inflated layout.
In `@app/src/main/java/com/limelight/DualScreenControlPanel.kt`:
- Around line 301-310: The fps, latency, and packetLoss formatters currently
force Locale.US instead of respecting the active device locale. Update these
methods in the metrics formatter to use Locale.getDefault() (or Android’s
locale-aware formatting), and add a test that sets a non-US locale and verifies
the localized decimal separator.
In `@app/src/main/java/com/limelight/ExternalDisplayManager.kt`:
- Line 375: Update the model selection around the target check in
ExternalDisplayManager so remote sources are identified by a URI scheme rather
than the broad startsWith("http") prefix; only pass a String for valid HTTP(S)
targets, while treating local paths such as "httpcache/bg.png" as File
instances.
In `@app/src/main/java/com/limelight/Game.kt`:
- Around line 2388-2395: Update the GameMenu construction in Game.kt so
hostContext and hostWindowToken use the same source: only use hostView.context
when hostView.windowToken is non-null, and otherwise fall back to the activity
context (this) with a null token. Preserve the existing hostWindowToken value
when the host view is attached.
In `@app/src/main/java/com/limelight/preferences/ExternalDisplayPreference.kt`:
- Around line 34-35: Update ExternalDisplayPreference’s initialize and lifecycle
methods to register a DisplayManager.DisplayListener for the preference
lifetime, invoking updateSummary() on display added, removed, or changed
callbacks. Unregister the same listener when the preference detaches, preserving
existing summary refresh behavior.
- Around line 92-96: Update ExternalDisplayPreference.updateSummary() so the
displays.size < 2 path only updates the summary and sets isEnabled to false; do
not assign isChecked, since it persists the use_external_display preference.
Apply the same change to the exception path while preserving its existing
summary and disabled-state behavior.
In `@app/src/main/java/com/limelight/preferences/StreamSettings.kt`:
- Around line 4264-4270: Update loadBackgroundImage() to increment a monotonic
load generation whenever a new background load starts, and pass that generation
through both the cached/network and worker-based paths. In
tryCachedThenNetwork() and loadBackgroundImageFromSource(), check that the
generation still matches before recursing to fallbacks and before applying any
UI result via runOnUiThread, preventing stale loads from replacing newer images.
In `@app/src/main/res/layout/dual_screen_control_panel.xml`:
- Around line 68-93: Update the header LinearLayout containing
dualScreenSessionTitle and dualScreenTargetDisplay so the session and target
columns use bounded widths instead of wrap_content, while preserving their
existing ellipsize behavior. Ensure both dynamic labels fit within their
assigned bounds and do not push the weighted status column off screen.
---
Nitpick comments:
In `@app/src/main/java/com/limelight/ExternalDisplayManager.kt`:
- Around line 227-239: Deduplicate the repeated selected-stream-display log in
the surrounding ExternalDisplayManager flow by moving the identical LimeLog.info
call before the streamDisplay.displayId branch. Keep the branch-specific
behavior unchanged: startExternalDisplayPresentation(streamDisplay) and return
only for non-default displays, while default displays continue through the
existing path.
In `@app/src/main/res/drawable/dual_screen_danger_bg.xml`:
- Around line 5-11: Replace the inline fill colors in the dual_screen_danger_bg
drawable with named color references. Add dual_screen_danger_pressed and
dual_screen_danger_background to advance_setting_colors.xml using the existing
hex values, then reference those resources for the corresponding shape fills
while leaving the stroke unchanged.
🪄 Autofix
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 Plus
Run ID: d60eb610-8a77-4e35-ab7e-fe45fe6b00db
📒 Files selected for processing (31)
.gitignoreapp/src/main/java/com/limelight/ConnectionCallbackHandler.ktapp/src/main/java/com/limelight/DualScreenControlPanel.ktapp/src/main/java/com/limelight/ExternalDisplayManager.ktapp/src/main/java/com/limelight/Game.ktapp/src/main/java/com/limelight/TargetDisplayResolver.ktapp/src/main/java/com/limelight/binding/input/advance_setting/KeyboardUIController.ktapp/src/main/java/com/limelight/gamemenu/GameMenu.ktapp/src/main/java/com/limelight/preferences/ExternalDisplayPreference.ktapp/src/main/java/com/limelight/preferences/StreamSettings.ktapp/src/main/res/drawable/dual_screen_action_bg.xmlapp/src/main/res/drawable/dual_screen_danger_bg.xmlapp/src/main/res/drawable/dual_screen_metric_bg.xmlapp/src/main/res/drawable/dual_screen_status_dot.xmlapp/src/main/res/layout/activity_game.xmlapp/src/main/res/layout/dual_screen_control_panel.xmlapp/src/main/res/layout/dual_screen_idle_background.xmlapp/src/main/res/layout/dual_screen_metric_bandwidth.xmlapp/src/main/res/layout/dual_screen_metric_battery.xmlapp/src/main/res/layout/dual_screen_metric_decode.xmlapp/src/main/res/layout/dual_screen_metric_fps.xmlapp/src/main/res/layout/dual_screen_metric_loss.xmlapp/src/main/res/layout/dual_screen_metric_resolution.xmlapp/src/main/res/layout/dual_screen_metric_rtt.xmlapp/src/main/res/layout/layer_6_keyboard.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values/advance_setting_colors.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/styles.xmlapp/src/test/java/com/limelight/DisplaySelectionPolicyTest.ktapp/src/test/java/com/limelight/DualScreenMetricFormatterTest.kt
Comment on lines
+137
to
+139
| keyboardLayout.findViewById<View>(R.id.btn_keyboard_resize).visibility = View.GONE | ||
| keyboardLayout.findViewById<View>(R.id.keyboard_resize_handle).visibility = View.GONE | ||
| keyboardLayout.findViewById<View>(R.id.keyboard_opacity_label).visibility = View.GONE |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use safe lookups for the optional controls.
Line 311 fetches R.id.btn_keyboard_resize as nullable and null-checks it at line 370. Here the same view is dereferenced directly. If the inflated layout omits the view, the constructor throws a NullPointerException. Use safe calls for all three lookups.
🐛 Proposed fix
- keyboardLayout.findViewById<View>(R.id.btn_keyboard_resize).visibility = View.GONE
- keyboardLayout.findViewById<View>(R.id.keyboard_resize_handle).visibility = View.GONE
- keyboardLayout.findViewById<View>(R.id.keyboard_opacity_label).visibility = View.GONE
+ keyboardLayout.findViewById<View?>(R.id.btn_keyboard_resize)?.visibility = View.GONE
+ keyboardLayout.findViewById<View?>(R.id.keyboard_resize_handle)?.visibility = View.GONE
+ keyboardLayout.findViewById<View?>(R.id.keyboard_opacity_label)?.visibility = View.GONE🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/limelight/binding/input/advance_setting/KeyboardUIController.kt`
around lines 137 - 139, Update the three visibility assignments in the keyboard
UI initialization to use nullable-safe lookups for btn_keyboard_resize,
keyboard_resize_handle, and keyboard_opacity_label, preventing dereferences when
optional controls are absent from the inflated layout.
Comment on lines
+301
to
+310
| fun fps(renderedFps: Float, receivedFps: Float): String = | ||
| String.format(Locale.US, "%.1f / %.1f", renderedFps, receivedFps) | ||
|
|
||
| fun rtt(rttInfo: Long): String = "${(rttInfo shr 32).toInt().coerceAtLeast(0)} ms" | ||
|
|
||
| fun latency(milliseconds: Float): String = | ||
| String.format(Locale.US, "%.1f ms", milliseconds.coerceAtLeast(0f)) | ||
|
|
||
| fun packetLoss(rate: Float): String = | ||
| String.format(Locale.US, "%.2f%%", rate.coerceAtLeast(0f)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
fd -a 'DualScreenControlPanel\.kt$' . || true
echo "== file excerpt =="
file="$(fd 'DualScreenControlPanel\.kt$' . | head -n1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '260,330p' "$file" | nl -ba -v260
fi
echo "== locale/localization usages nearby / project-wide numeric formatting =="
rg -n "String\.format|Locale\.US|Locale\.getDefault|DecimalFormat|NumberFormat|r24|`@android`:style/TextAppearance|`@android`:style/TextAppearance|res/values" -S . -g '*.kotlin' -g '*.kt' -g '*.xml' -g '*.java' | head -200 || trueRepository: qiin2333/moonlight-vplus
Length of output: 384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '260,330p' /home/jailuser/git/app/src/main/java/com/limelight/DualScreenControlPanel.kt
echo "== numeric formatting usages =="
rg -n "String\.format|Locale\.US|Locale\.getDefault|NumberFormat|DecimalFormat|String\.format\(Locale" /home/jailuser/git/app/src/main/java/com/limelight /home/jailuser/git/app/src/main/java -S 2>/dev/null | head -200 || true
echo "== deterministic locale formatting probe =="
python3 - <<'PY'
from locale import setlocale, LC_NUMERIC, LC_ALL, C
for locale in ('en_US', 'de_DE', 'ar_SA'):
try:
setlocale(LC_NUMERIC, locale)
except Exception as e:
print(f"{locale}: unavailable: {e}")
continue
for value in (9.876, 9.8):
print(f"{locale}; value={value}; default={value:.1f} vs US={value:.1f}")
PYRepository: qiin2333/moonlight-vplus
Length of output: 35499
Use the active device locale for formatted metrics.
String.format(Locale.US, ...) forces a period decimal separator for FPS, latency, and packet loss output. Let these user-facing values use Locale.getDefault() or Android string formatting so non-US numeric locales are respected. Add a formatter test with a non-US locale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/limelight/DualScreenControlPanel.kt` around lines 301 -
310, The fps, latency, and packetLoss formatters currently force Locale.US
instead of respecting the active device locale. Update these methods in the
metrics formatter to use Locale.getDefault() (or Android’s locale-aware
formatting), and add a test that sets a non-US locale and verifies the localized
decimal separator.
added 6 commits
August 4, 2026 19:57
This was referenced Aug 8, 2026
qiin2333
force-pushed
the
master
branch
2 times, most recently
from
August 25, 2026 03:00
85b5eb3 to
a4d7ce0
Compare
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
1 participant
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.
Summary
Compatibility
This implementation contains no Thor-specific identifiers. It uses Android’s standard
DisplayManager,Presentation, multi-display window tokens, and input event APIs. Dual-screen mode is opt-in, and users explicitly select the primary streaming display from the displays reported by Android.Devices that do not expose both panels as independent Android displays, or whose firmware restricts secondary-display presentations, navigation gestures, or input dispatch, may require separate handling.
Testing
Lint
:app:lintNonRootDebugreports 104 warnings and 1 hint. The repository baseline filters 133 existing errors, 1579 warnings, and 1 hint. These changes introduce no new lint errors.provided by chat-gpt 5.6sol-maxhigh
Summary by CodeRabbit
New Features
Bug Fixes
Tests