Skip to content

Conversation

@SapphireRhodonite
Copy link

@SapphireRhodonite SapphireRhodonite commented Jan 7, 2026

Added Support to use the external screen as a Touchpad, Keyboard, or Touchpad+Keyboard

Summary by CodeRabbit

  • New Features
    • External display input modes: Off, Touchpad, Keyboard, Hybrid (touchpad+keyboard)
    • On-screen keyboard, hybrid touchpad+keyboard UI, and an overlay for external displays
    • External view swapping to move the game between internal and external displays
    • Settings added to configure and persist external display mode and swap behavior
    • New UI strings and color themes for external-display components

✏️ Tip: You can customize this high-level summary in your review settings.

SapphireRhodonite and others added 7 commits December 15, 2025 10:40
This commit refactors the external display input handling.

It removes the `ExternalKeyboardView` and replaces it with a new `ExternalOnScreenKeyboardView` for a cleaner implementation. The `winHandler` and `inputControlsViewProvider` dependencies have been removed from the `ExternalDisplayInputController` and related classes.

The "Hybrid" mode UI is updated to use a floating action button to toggle the on-screen keyboard visibility over the touchpad, instead of switching between two full-screen views.
# Conflicts:
#	app/src/main/res/values/strings.xml
@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

Adds external-display input and swap support: new controllers and views for rendering/input on secondary displays, UI settings and resources, preferences/container persistence, and integration into XServerScreen with lifecycle and overlay handling.

Changes

Cohort / File(s) Summary
Preferences & Persistence
app/src/main/java/app/gamenative/PrefManager.kt, app/src/main/java/com/winlator/container/Container.java, app/src/main/java/com/winlator/container/ContainerData.kt, app/src/main/java/app/gamenative/utils/ContainerUtils.kt
Added preference keys/properties for external display input mode and swap; added Container constants/getters/setters and ContainerData fields for externalDisplayMode/externalDisplaySwap; wired defaults, save/load, and Pref ↔ Container synchronization.
External display input controllers
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt, app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt
New controllers to manage external input presentation lifecycle and to swap the game view to/from an external display; register DisplayListener, select target display, create/dismiss Presentations, and move XServerView between internal/external hosts.
On-screen keyboard & overlay UI
app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt, app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt
New on-screen keyboard view that emits key events to XServer and a SwapInputOverlayView that presents keyboard/hybrid hint, toggle, and layout handling for overlays.
App UI integration
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt, app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Added external display mode dropdown and swap toggle in container config UI; XServerScreen now creates/starts/stops controllers, manages a game host container for layering, and routes touch events through overlay handling.
Resources
app/src/main/res/values/strings.xml, app/src/main/res/values/colors.xml
Added string resources for external display input labels and modes; added colors for external display surfaces, keyboard, keys, and highlights.

Sequence Diagram(s)

sequenceDiagram
  participant UI as ContainerConfigDialog
  participant Screen as XServerScreen
  participant Controller as ExternalDisplayInputController
  participant SwapCtrl as ExternalDisplaySwapController
  participant DisplayMgr as Android DisplayManager
  participant XServer as XServer
  participant Touchpad as TouchpadView

  UI->>Screen: user selects externalDisplayMode / toggle swap
  Screen->>Controller: setMode(mode)
  Screen->>SwapCtrl: setSwapEnabled(enabled)
  Controller->>DisplayMgr: register DisplayListener / query displays
  SwapCtrl->>DisplayMgr: register DisplayListener / query displays
  DisplayMgr-->>Controller: display added/removed/changed
  Controller->>Controller: create/dismiss ExternalInputPresentation
  Controller->>XServer: bind input target (touch/keyboard)
  Controller->>Touchpad: request TouchpadView via provider
  Touchpad->>XServer: deliver touch events
  DisplayMgr-->>SwapCtrl: display added/removed/changed
  SwapCtrl->>SwapCtrl: create/dismiss GamePresentation
  SwapCtrl->>XServer: move XServerView to external/internal host
  SwapCtrl->>Screen: onGameOnExternalChanged(callback)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Emuready #121 — modifies ContainerData/ContainerUtils persistence and defaults; similar container-field plumbing patterns.
  • Fix env vars #230 — edits PrefManager and defaults wiring for new configuration fields; overlaps with preference/container integration.

Poem

🐰
I hopped to the screen and gave a tap,
A keyboard bloomed near my leaf-laptop lap.
Touchpads glide, hybrid toggles play,
Displays whisper input far away.
Hooray — little hops made input sway! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.35% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Dual screen support' accurately reflects the main objective of the PR, which adds external screen support for use as Touchpad, Keyboard, or combined Touchpad+Keyboard input modes.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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/container/ContainerData.kt (1)

75-84: Align externalDisplayMode defaults across property and Saver.restore

externalDisplayMode defaults to "hybrid" in the data class, but Saver.restore falls back to "touchpad" when the key is missing:

val externalDisplayMode: String = "hybrid",
...
externalDisplayMode = (savedMap["externalDisplayMode"] as? String) ?: "touchpad",

Given PrefManager.externalDisplayInputMode also defaults to "hybrid", this creates two effective defaults depending on the code path (fresh ContainerData() vs restored state / older saved maps). If "hybrid" is meant to be the canonical default, consider using it in restore as well, or add a brief comment explaining why "touchpad" is preferred here.

Also applies to: 130-138, 183-191

🤖 Fix all issues with AI agents
In @app/src/main/java/com/winlator/container/Container.java:
- Around line 952-959: The getter getExternalDisplayMode, the setter
setExternalDisplayMode, and the externalDisplayMode field initializer use
inconsistent fallback values ("hybrid" vs "touchpad"); pick a single canonical
default (prefer "hybrid" to match the existing field initializer) and update
setExternalDisplayMode to use that default when null is passed, ensuring
getExternalDisplayMode, setExternalDisplayMode, and the externalDisplayMode
field all agree on the same fallback value.
🧹 Nitpick comments (3)
app/src/main/java/app/gamenative/PrefManager.kt (1)

426-430: External display input pref looks correct; consider centralizing allowed values

The key name and "hybrid" default fit existing prefs and the documented off|touchpad|keyboard|hybrid domain. To reduce drift with other callers (e.g., ContainerData.externalDisplayMode and ExternalDisplayInputController.fromConfig), consider centralizing the canonical string values in one place (or mapping via the controller’s enum) instead of repeating literals.

app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)

63-63: ExternalDisplayInputController lifecycle wiring is sound; consider making the no-op callback explicit

The controller is correctly scoped to the FrameLayout lifecycle: it’s created with the right context/xServer, starts immediately, and is stopped via an attach-state listener on view detach, which should ensure the display listener is unregistered.

The only nit is the empty onViewAttachedToWindow override:

override fun onViewAttachedToWindow(v: View) {}

Static analysis flags this as an empty function block. If you don’t need any attach-time behavior, consider making the no-op explicit (e.g., adding a // no-op comment or using = Unit) so the intent is clearer and the rule can be suppressed or configured accordingly.

Also applies to: 815-829

app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)

163-168: External display mode dropdown wiring is correct; consider sharing config-value mapping

The UI/state wiring for external display modes looks consistent:

  • Display labels come from externalDisplayModes (off, touchpad, keyboard, hybrid).
  • externalDisplayModeIndex is initialized from config.externalDisplayMode.lowercase() with a sensible fallback to 0 (“off”) for unknown values.
  • The SettingsListDropdown writes back to config.externalDisplayMode using the same index→string mapping.

Functionally this should behave as intended. To reduce duplication and the chance of typos drifting, you might introduce a single parallel list of config values, e.g.:

val externalDisplayConfigValues = listOf("off", "touchpad", "keyboard", "hybrid")
val externalDisplayModes = externalDisplayConfigValues.map { /* map to stringResource label */ }

Then:

  • Initialize externalDisplayModeIndex via externalDisplayConfigValues.indexOf(config.externalDisplayMode.lowercase()).coerceAtLeast(0).
  • On selection, write back externalDisplayMode = externalDisplayConfigValues[index].

That keeps the mapping in one place while preserving current behavior.

Also applies to: 687-695, 1722-1740

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2d30cc4 and a67515c.

📒 Files selected for processing (9)
  • app/src/main/java/app/gamenative/PrefManager.kt
  • app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt
  • app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/ContainerUtils.kt
  • app/src/main/java/com/winlator/container/Container.java
  • app/src/main/java/com/winlator/container/ContainerData.kt
  • app/src/main/res/values/strings.xml
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (2)
  • setMode (76-79)
  • start (63-66)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
app/src/main/java/app/gamenative/ui/component/settings/SettingsListDropdown.kt (1)
  • SettingsListDropdown (40-125)
app/src/main/java/app/gamenative/ui/theme/Color.kt (1)
  • settingsTileColors (27-32)
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (1)
app/src/main/java/com/winlator/widget/TouchpadView.java (1)
  • TouchpadView (28-728)
🪛 detekt (1.23.8)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt

[warning] 824-824: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)

🔇 Additional comments (15)
app/src/main/res/values/strings.xml (1)

524-529: LGTM!

The new string resources are well-structured and follow the existing naming conventions. The descriptions are clear and user-friendly.

app/src/main/java/com/winlator/container/Container.java (2)

115-116: LGTM!

The new field follows the existing pattern in the class and the default value of "hybrid" aligns with the feature design.


651-651: LGTM!

The persistence of externalDisplayMode follows the established pattern for other container fields.

app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt (4)

17-50: LGTM!

Well-structured Kotlin class with clean separation of concerns. The use of data classes for KeySpec and KeyButton, along with the ShiftState enum, makes the code readable and maintainable.


52-134: LGTM!

The keyboard layout implementation is well-structured with appropriate key weights for a balanced visual appearance. The five-row layout covers standard QWERTY keys plus modifiers and navigation arrows.


136-196: LGTM!

The key handling logic is well-implemented. The shift state management correctly handles the three states (OFF, ON, CAPS) with appropriate behavior for letters vs. symbols.


236-241: LGTM!

The tapKey method correctly handles shift key injection around the actual key press/release sequence. The implementation properly injects shift press before the key and releases it after.

app/src/main/java/app/gamenative/utils/ContainerUtils.kt (3)

124-124: LGTM!

The external display mode is correctly sourced from PrefManager.externalDisplayInputMode following the established pattern.


162-162: LGTM!

The persistence of externalDisplayMode to preferences follows the established pattern.


227-227: LGTM!

The external display mode is correctly wired through all three data flow points: reading from container, including in ContainerData construction, and applying back to container.

Also applies to: 270-270, 391-391

app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (5)

24-38: LGTM!

Clean class structure with well-defined Mode enum. The fromConfig companion function provides a safe default (OFF) for unknown or null values.


63-79: LGTM!

The lifecycle management is well-implemented. The start() method correctly registers the display listener on the main looper for thread safety, and stop() properly cleans up resources. The silent exception catch during unregistration is acceptable for cleanup operations.


81-121: LGTM!

The presentation management logic correctly handles mode changes and display availability. The findPresentationDisplay() appropriately filters for presentation-capable displays while excluding the default and hidden displays.


123-145: LGTM!

The presentation correctly sets FLAG_NOT_FOCUSABLE and FLAG_NOT_TOUCH_MODAL window flags to ensure the external display doesn't steal focus from the main window. This is essential for the feature's usability as noted in the PR objectives.


210-276: LGTM!

The HybridInputLayout implementation is well-designed. The floating action button toggle with dynamic positioning based on keyboard visibility provides a good UX. The keyboard starting hidden (line 233) and the toggle button repositioning logic (lines 269-275) work together smoothly.

@joshuatam
Copy link
Contributor

Cool, any device was tested to work?

@SapphireRhodonite
Copy link
Author

Cool, any device was tested to work?

Ayn Thor!

// Touchscreen mode
private boolean touchscreenMode = false;
// External display input handling
private String externalDisplayMode = "hybrid";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend creating a CONST for this that can be passed around so we're not using a hard-coded string everywhere for the different displayInput modes.

Copy link
Owner

Choose a reason for hiding this comment

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

Correct. Also it shouldn't be hybrid by default right?

Comment on lines 689 to 691
"touchpad" -> 1
"keyboard" -> 2
"hybrid" -> 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Comment on lines 21 to 22
private const val EXTERNAL_TOUCHPAD_BG: Int = 0xFF2B2B2B.toInt()
private const val EXTERNAL_KEYBOARD_BG: Int = 0xFF2B2B2B.toInt()
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good for you to check if we have a theme/colors file and bring it in from there rather than hardcode the hexvalue

Copy link
Owner

Choose a reason for hiding this comment

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

Correct, we do have colors constants

@phobos665
Copy link
Contributor

phobos665 commented Jan 8, 2026

Overall looks good, just added a few QoL changes for maintainability. Will do a final test with my Thor tonight

@phobos665
Copy link
Contributor

Tested on the Thor, looks and works great.

I think we can iterate and adjust it as we go based on any community feedback.

Thanks for the contribution!

Comment on lines 815 to 829
val externalDisplayController = ExternalDisplayInputController(
context = context,
xServer = xServerView.getxServer(),
touchpadViewProvider = { PluviaApp.touchpadView },
).apply {
setMode(ExternalDisplayInputController.fromConfig(container.externalDisplayMode))
start()
}
frameLayout.addOnAttachStateChangeListener(object : View.OnAttachStateChangeListener {
override fun onViewAttachedToWindow(v: View) {}

override fun onViewDetachedFromWindow(v: View) {
externalDisplayController.stop()
}
})
Copy link
Owner

Choose a reason for hiding this comment

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

Should this be in an if statement? We shouldn't do this if there's only a single screen, right?

externaldisplay: add screen swap support
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Around line 122-125: The createNewContainer fallback instantiations of
ContainerData are missing propagation of the user's external display defaults;
update every ContainerData(...) created in createNewContainer (and other
fallback sites where ContainerData is constructed) to include
externalDisplayMode = PrefManager.externalDisplayInputMode and
externalDisplaySwap = PrefManager.externalDisplaySwap so newly created
containers inherit those preferences (references: createNewContainer,
ContainerData, PrefManager.externalDisplayInputMode,
PrefManager.externalDisplaySwap).
🧹 Nitpick comments (7)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)

893-895: Remove the empty attach callback body.
Detekt flags this as an empty block; prefer an expression body.

♻️ Suggested cleanup
 frameLayout.addOnAttachStateChangeListener(object : View.OnAttachStateChangeListener {
-    override fun onViewAttachedToWindow(v: View) {}
+    override fun onViewAttachedToWindow(v: View) = Unit
app/src/main/java/app/gamenative/externaldisplay/SwapInputOverlayView.kt (2)

82-104: TOUCHPAD mode falls through to generic else branch.

The Mode enum has four values (OFF, TOUCHPAD, KEYBOARD, HYBRID), but setMode only explicitly handles KEYBOARD and HYBRID. TOUCHPAD falls through to the else branch alongside OFF. If TOUCHPAD mode should behave differently than OFF in this overlay context, consider adding an explicit case.

If the current behavior is intentional (TOUCHPAD hides all overlay elements), adding a brief comment would improve clarity.


106-118: Duplicated toggle/position logic with HybridInputLayout.

The toggleKeyboard() and updateToggleButtonPosition() methods are nearly identical to those in HybridInputLayout (ExternalDisplayInputController.kt, lines 266-277). Consider extracting a shared helper or base class to reduce duplication if these views evolve together.

This is a minor observation given the "Chill" review mode—acceptable to defer.

app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt (1)

160-174: setTextSize(16f) uses scaled pixels by default, but returning false from touch listener may cause issues.

Button.setTextSize(float) interprets the value as SP by default, so that's fine. However, returning false from the OnTouchListener allows the button's default click handling to also run, which may cause double-handling in some edge cases. Since you're manually handling ACTION_DOWN/ACTION_UP, consider returning true to consume the event.

Suggested change
                 setOnTouchListener { _, event ->
                     handleKeyTouch(spec, event)
-                    false
+                    true
                 }
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplaySwapController.kt (2)

119-126: Duplicated findPresentationDisplay() logic.

This method is identical to the one in ExternalDisplayInputController.kt (lines 115-123). Consider extracting to a shared utility function to avoid duplication and ensure consistent display selection logic across both controllers.

Example extraction
// In a shared utility file or companion object
fun findPresentationDisplay(context: Context, displayManager: DisplayManager?): Display? {
    val currentDisplay = context.display ?: return null
    return displayManager
        ?.getDisplays(DisplayManager.DISPLAY_CATEGORY_PRESENTATION)
        ?.firstOrNull { display ->
            display.displayId != currentDisplay.displayId && display.name != "HiddenDisplay"
        }
}

47-53: Silently swallowing exception in stop().

The empty catch block could hide unexpected issues. Consider at least logging the exception for debugging purposes.

Suggested change
     fun stop() {
         dismissPresentation()
         try {
             displayManager?.unregisterDisplayListener(displayListener)
-        } catch (_: Exception) {
+        } catch (e: Exception) {
+            android.util.Log.w("ExternalDisplaySwap", "Failed to unregister listener", e)
         }
     }
app/src/main/java/app/gamenative/externaldisplay/ExternalDisplayInputController.kt (1)

70-76: Silent exception swallow in stop()—same pattern as SwapController.

Consider logging the exception for consistency and debuggability, similar to the suggestion for ExternalDisplaySwapController.

Comment on lines +122 to +125
dinputMapperType = PrefManager.dinputMapperType.toByte(),
disableMouseInput = PrefManager.disableMouseInput,
externalDisplayMode = PrefManager.externalDisplayInputMode,
externalDisplaySwap = PrefManager.externalDisplaySwap,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Propagate external display defaults into new container creation.
You’ve wired the defaults and container mapping, but the createNewContainer fallback ContainerData(...) still omits externalDisplayMode/externalDisplaySwap. This means user defaults won’t apply to newly created containers.

🛠️ Proposed fix (createNewContainer default ContainerData)
             ContainerData(
                 screenSize = PrefManager.screenSize,
                 envVars = PrefManager.envVars,
                 cpuList = PrefManager.cpuList,
                 cpuListWoW64 = PrefManager.cpuListWoW64,
                 graphicsDriver = PrefManager.graphicsDriver,
                 graphicsDriverVersion = PrefManager.graphicsDriverVersion,
                 graphicsDriverConfig = PrefManager.graphicsDriverConfig,
                 dxwrapper = initialDxWrapper,
                 dxwrapperConfig = PrefManager.dxWrapperConfig,
                 audioDriver = PrefManager.audioDriver,
                 wincomponents = PrefManager.winComponents,
                 drives = drives,
                 execArgs = PrefManager.execArgs,
                 showFPS = PrefManager.showFps,
                 launchRealSteam = PrefManager.launchRealSteam,
                 wow64Mode = PrefManager.wow64Mode,
                 startupSelection = PrefManager.startupSelection.toByte(),
                 box86Version = PrefManager.box86Version,
                 box64Version = PrefManager.box64Version,
                 box86Preset = PrefManager.box86Preset,
                 box64Preset = PrefManager.box64Preset,
                 desktopTheme = WineThemeManager.DEFAULT_DESKTOP_THEME,
                 language = PrefManager.containerLanguage,
                 containerVariant = PrefManager.containerVariant,
                 wineVersion = PrefManager.wineVersion,
                 emulator = PrefManager.emulator,
                 fexcoreVersion = PrefManager.fexcoreVersion,
                 fexcoreTSOMode = PrefManager.fexcoreTSOMode,
                 fexcoreX87Mode = PrefManager.fexcoreX87Mode,
                 fexcoreMultiBlock = PrefManager.fexcoreMultiBlock,
                 fexcorePreset = PrefManager.fexcorePreset,
                 renderer = PrefManager.renderer,
                 csmt = PrefManager.csmt,
                 videoPciDeviceID = PrefManager.videoPciDeviceID,
                 offScreenRenderingMode = PrefManager.offScreenRenderingMode,
                 strictShaderMath = PrefManager.strictShaderMath,
                 useDRI3 = PrefManager.useDRI3,
                 videoMemorySize = PrefManager.videoMemorySize,
                 mouseWarpOverride = PrefManager.mouseWarpOverride,
                 enableXInput = PrefManager.xinputEnabled,
                 enableDInput = PrefManager.dinputEnabled,
                 dinputMapperType = PrefManager.dinputMapperType.toByte(),
                 disableMouseInput = PrefManager.disableMouseInput,
+                externalDisplayMode = PrefManager.externalDisplayInputMode,
+                externalDisplaySwap = PrefManager.externalDisplaySwap,
             )

Also applies to: 163-164, 229-230, 273-274, 395-396

🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt` around lines 122 -
125, The createNewContainer fallback instantiations of ContainerData are missing
propagation of the user's external display defaults; update every
ContainerData(...) created in createNewContainer (and other fallback sites where
ContainerData is constructed) to include externalDisplayMode =
PrefManager.externalDisplayInputMode and externalDisplaySwap =
PrefManager.externalDisplaySwap so newly created containers inherit those
preferences (references: createNewContainer, ContainerData,
PrefManager.externalDisplayInputMode, PrefManager.externalDisplaySwap).

Copy link

@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.

2 issues found across 12 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt">

<violation number="1" location="app/src/main/java/app/gamenative/externaldisplay/ExternalOnScreenKeyboardView.kt:65">
P2: Move the on-screen key labels to string resources instead of hardcoding them in code so the keyboard can be localized and managed consistently.</violation>
</file>

<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:525">
P2: Keyboard-only external display mode still allows touchpad input because unhandled overlay touches always fall through to `touchpadView`. This makes Mode.KEYBOARD behave like hybrid when the overlay is visible. Consider consuming all touches in keyboard-only mode (e.g., make the overlay swallow events or gate touchpad forwarding by the configured mode).</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
  • 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.

private fun buildLayout() {
addRow(
listOf(
KeySpec("Esc", keycode = XKeycode.KEY_ESC, weight = 1.25f, action = Action.ESC),
Copy link

Choose a reason for hiding this comment

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

P2: Move the on-screen key labels to string resources instead of hardcoding them in code so the keyboard can be localized and managed consistently.

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/externaldisplay/ExternalOnScreenKeyboardView.kt, line 65:

<comment>Move the on-screen key labels to string resources instead of hardcoding them in code so the keyboard can be localized and managed consistently.</comment>

<file context>
@@ -0,0 +1,330 @@
+    private fun buildLayout() {
+        addRow(
+            listOf(
+                KeySpec("Esc", keycode = XKeycode.KEY_ESC, weight = 1.25f, action = Action.ESC),
+                KeySpec("1", "!", XKeycode.KEY_1),
+                KeySpec("2", "@", XKeycode.KEY_2),
</file context>

// If controls didn't handle it or aren't visible, send to touchMouse
if (!controlsHandled) {
PluviaApp.touchpadView?.onTouchEvent(it)
PluviaApp.touchpadView?.onTouchEvent(event)
Copy link

Choose a reason for hiding this comment

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

P2: Keyboard-only external display mode still allows touchpad input because unhandled overlay touches always fall through to touchpadView. This makes Mode.KEYBOARD behave like hybrid when the overlay is visible. Consider consuming all touches in keyboard-only mode (e.g., make the overlay swallow events or gate touchpad forwarding by the configured mode).

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/screen/xserver/XServerScreen.kt, line 525:

<comment>Keyboard-only external display mode still allows touchpad input because unhandled overlay touches always fall through to `touchpadView`. This makes Mode.KEYBOARD behave like hybrid when the overlay is visible. Consider consuming all touches in keyboard-only mode (e.g., make the overlay swallow events or gate touchpad forwarding by the configured mode).</comment>

<file context>
@@ -499,17 +507,22 @@ fun XServerScreen(
                 // If controls didn't handle it or aren't visible, send to touchMouse
                 if (!controlsHandled) {
-                    PluviaApp.touchpadView?.onTouchEvent(it)
+                    PluviaApp.touchpadView?.onTouchEvent(event)
                 }
 
</file context>

Comment on lines +524 to +531
<string name="external_display_input">External Display Input</string>
<string name="external_display_input_subtitle">Choose how a connected presentation display should behave</string>
<string name="external_display_mode_off">Do not use external display</string>
<string name="external_display_mode_touchpad">Use as touchpad surface</string>
<string name="external_display_mode_keyboard">Use as full keyboard</string>
<string name="external_display_mode_hybrid">Hybrid (touchpad + keyboard button)</string>
<string name="external_display_swap">Show Game on External Display</string>
<string name="external_display_swap_subtitle">Swap screens so the game renders on the external display and this device becomes the controller surface</string>
Copy link
Owner

Choose a reason for hiding this comment

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

I'll add translations for all the other languages and make a PR to your branch

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.

4 participants