Added better support for pocket taco/8bitdo flippad type portrait con…#541
Added better support for pocket taco/8bitdo flippad type portrait con…#541utkarshdalal merged 1 commit intomasterfrom
Conversation
📝 WalkthroughWalkthroughIntroduces orientation-aware UI layout scaffolding to XServerScreen by conditionally creating portrait (vertical LinearLayout) or landscape (FrameLayout) containers, repositioning InputControlsView placement based on orientation, and routing touch events through gameRoot state in portrait mode while retaining previous behavior in landscape. Changes
Sequence DiagramsequenceDiagram
participant User
participant XServerScreen
participant Config as Configuration
participant Layout as Layout Container
participant InputControls as InputControlsView
participant GameRoot as gameRoot
User->>XServerScreen: Render with orientation
XServerScreen->>Config: Check isPortrait
alt Portrait Mode
Config-->>XServerScreen: isPortrait = true
XServerScreen->>Layout: Create vertical LinearLayout as mainRoot
XServerScreen->>GameRoot: Assign gameRoot reference
XServerScreen->>Layout: Create bottom controlsContainer
XServerScreen->>InputControls: Place in controlsContainer (bottom-fixed)
User->>InputControls: Touch input
InputControls->>GameRoot: Route via gameRoot.dispatchTouchEvent()
else Landscape Mode
Config-->>XServerScreen: isPortrait = false
XServerScreen->>Layout: Use FrameLayout as mainRoot
XServerScreen->>InputControls: Add as overlay to frameLayout
User->>InputControls: Touch input
InputControls->>Layout: Handle directly (previous behavior)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
654-672:⚠️ Potential issue | 🟡 MinorPortrait mode may double-dispatch touch events to
swapInputOverlay.When the overlay is visible but doesn't consume the event (line 657 returns
false), the code falls through to line 661 which dispatches togameRoot. Since the overlay is a descendant ofgameRoot(added at line 1013 insideframeLayout), Android's view dispatch will attempt to deliver the same event to the overlay again.In landscape mode this isn't an issue because the fallthrough path (lines 663–670) manually routes to specific views, skipping the overlay. Consider adding the same manual routing for portrait, or skipping the overlay in
gameRootdispatch:Suggested approach
if (isPortrait) { - gameRoot?.dispatchTouchEvent(event) + val controlsHandled = if (areControlsVisible) { + PluviaApp.inputControlsView?.onTouchEvent(event) ?: false + } else { + false + } + if (!controlsHandled) { + PluviaApp.touchpadView?.onTouchEvent(event) + }Alternatively, if
gameRoot?.dispatchTouchEventis the desired approach for coordinate-based routing, consider making the overlay intercept check exhaustive (i.e., always returntruefrompointerInteropFilterbefore reachinggameRootif overlay is visible, even if overlay didn't consume).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 654 - 672, The pointerInteropFilter can double-dispatch when swapInputOverlay is visible but returns false; modify the pointerInteropFilter in XServerScreen so that when swapInputOverlay?.visibility == View.VISIBLE you do not fall through to gameRoot: call swapInputOverlay?.dispatchTouchEvent(event) as you do now, and if it returns true return true, otherwise also return true (consume the event) to prevent gameRoot?.dispatchTouchEvent(event) from running; reference swapInputOverlay, pointerInteropFilter, gameRoot and isPortrait and keep the existing landscape routing unchanged (or alternatively replace the portrait path with manual routing to specific controls if you prefer coordinate-based dispatch).
🤖 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/screen/xserver/XServerScreen.kt`:
- Line 647: The factory-created view hierarchy inside AndroidView can become
out-of-sync with the composable-scoped isPortrait used by pointerInteropFilter
because the factory lambda (where context.resources.configuration.orientation is
captured) only runs once; update the code to ensure the view is recreated when
orientation changes by wrapping the AndroidView in a key(isPortrait) (or
otherwise trigger re-creation) so the factory rebuilds the correct layout, or
refactor the view construction to be orientation-adaptive; reference isPortrait,
the factory lambda that uses context.resources.configuration.orientation,
AndroidView, pointerInteropFilter, and PrefManager.allowedOrientation when
making the change.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 654-672: The pointerInteropFilter can double-dispatch when
swapInputOverlay is visible but returns false; modify the pointerInteropFilter
in XServerScreen so that when swapInputOverlay?.visibility == View.VISIBLE you
do not fall through to gameRoot: call
swapInputOverlay?.dispatchTouchEvent(event) as you do now, and if it returns
true return true, otherwise also return true (consume the event) to prevent
gameRoot?.dispatchTouchEvent(event) from running; reference swapInputOverlay,
pointerInteropFilter, gameRoot and isPortrait and keep the existing landscape
routing unchanged (or alternatively replace the portrait path with manual
routing to specific controls if you prefer coordinate-based dispatch).
| } | ||
| } | ||
|
|
||
| val isPortrait = LocalConfiguration.current.orientation == Configuration.ORIENTATION_PORTRAIT |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, search for orientation locking mechanisms
rg -n "SetAllowedOrientation\|screenOrientation\|SCREEN_ORIENTATION\|setRequestedOrientation" --type=kt -C3Repository: utkarshdalal/GameNative
Length of output: 93
🏁 Script executed:
# Also check the context around lines 647 and 676
sed -n '640,680p' app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktRepository: utkarshdalal/GameNative
Length of output: 1977
🏁 Script executed:
# Check if there are activity orientation policies or AndroidView key recomposition
rg -n "AndroidView\|DisposableEffect\|key(" app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt | head -20Repository: utkarshdalal/GameNative
Length of output: 187
🏁 Script executed:
# Search for orientation locking with correct syntax
rg "SetAllowedOrientation|screenOrientation|SCREEN_ORIENTATION|setRequestedOrientation" -C3Repository: utkarshdalal/GameNative
Length of output: 7612
🏁 Script executed:
# Search for AndroidView and key usage
rg "AndroidView" -A2 -B2 | head -30Repository: utkarshdalal/GameNative
Length of output: 3199
🏁 Script executed:
# Check the activity configuration settings
rg "android:screenOrientation" --type=xmlRepository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Look for AndroidManifest entries for this activity
fd "AndroidManifest.xml" -x grep -n "xserver\|XServerScreen" {} +Repository: utkarshdalal/GameNative
Length of output: 136
🏁 Script executed:
# Check the setOrientationTo implementation
rg "fun setOrientationTo" -A15Repository: utkarshdalal/GameNative
Length of output: 1616
🏁 Script executed:
# Check PrefManager.allowedOrientation
rg "allowedOrientation" -B2 -A2 | head -40Repository: utkarshdalal/GameNative
Length of output: 2565
🏁 Script executed:
# Look for DisposableEffect or other recomposition triggers around AndroidView
rg "DisposableEffect|LaunchedEffect" app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt -B3 -A5Repository: utkarshdalal/GameNative
Length of output: 1708
The isPortrait variable in the factory can diverge from the composable scope if device orientation changes during gameplay.
Line 647 reads LocalConfiguration.current.orientation (reactive, updates on recomposition), while line 676 captures context.resources.configuration.orientation once in the factory lambda. Since AndroidView's factory is invoked only once at creation, the view hierarchy (e.g., LinearLayout for portrait vs FrameLayout for landscape) is determined at that moment. If the device orientation changes after factory creation—which is possible since PrefManager.allowedOrientation is an EnumSet<Orientation> that can include multiple values and the app respects device rotation within allowed orientations—the pointerInteropFilter (line 660) will dispatch touch events according to the updated isPortrait value from the composable scope, but the underlying view structure built in the factory will not adapt.
To ensure the view hierarchy matches the touch dispatch logic, either wrap the AndroidView in a key(isPortrait) to force recreation on orientation change, or redesign the layout to be orientation-adaptive rather than structure-dependent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` at line
647, The factory-created view hierarchy inside AndroidView can become
out-of-sync with the composable-scoped isPortrait used by pointerInteropFilter
because the factory lambda (where context.resources.configuration.orientation is
captured) only runs once; update the code to ensure the view is recreated when
orientation changes by wrapping the AndroidView in a key(isPortrait) (or
otherwise trigger re-creation) so the factory rebuilds the correct layout, or
refactor the view construction to be orientation-adaptive; reference isPortrait,
the factory lambda that uses context.resources.configuration.orientation,
AndroidView, pointerInteropFilter, and PrefManager.allowedOrientation when
making the change.
1 participant
…trollers
Summary by cubic
Adds a portrait-specific layout and touch routing to better support Pocket TACO/8BitDo FlipPad-style controllers. Game stays on top; controls live in a fixed-height bottom panel for a more consistent feel.
Written for commit 8e97aa3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes