Skip to content

fix(overlay): remove extra Guide shortcut handlers - #564

Open
morheddsb wants to merge 1 commit into
ShadowBlip:mainfrom
morheddsb:fix/overlay-guide-shortcut-cleanup
Open

morheddsb wants to merge 1 commit into
ShadowBlip:mainfrom
morheddsb:fix/overlay-guide-shortcut-cleanup

Conversation

@morheddsb

@morheddsb morheddsb commented Oct 6, 2026 •

Copy link
Copy Markdown

Summary

Fix overlay-mode Guide shortcut release handling while preserving existing shortcuts and controller mappings.

  • Replace unreachable release checks with a small loop that clears active synthetic actions when the Guide-combo marker ends. This covers keyboard, QAM, capture, screenshot,
    and Quick Bar actions, including when Guide is released first.
  • Fix the screenshot cleanup branch to check for RB release rather than RB press.
  • Preserve the Guide+A, Guide+X, Guide+Y capture-placeholder, and Guide+RB handlers, including screenshot forwarding.
  • Preserve the original LeftTop/RightTop shortcut aliases, retaining Quick Bar access for devices without a dedicated second QAM button.
  • Keep Guide+B, QAM2, dedicated Keyboard/QAM handling, and normal menu navigation.
  • Correct the Guide+B documentation example and remove the profile’s trailing JSON comma.
  • Add three focused Godot regression tests.

Why

Refs ShadowBlip/OpenGamepadUI#549.
Related discussion: ShadowBlip/InputPlumber#747.

While reverting the earlier shortcut removals following review feedback, I noticed two pre-existing issues in OverlayInputManager:

  1. The Guide-action release block required the same InputEventAction to match both the Guide-combo marker and another button’s release. Those nested checks could not match,
    leaving the intended cleanup unreachable.
  2. The screenshot release branch checked is_action_pressed("ogui_rb_ov") inside an event.is_released() branch, so an ordinary RB-release event did not clear the
    screenshot action through that path.

These fixes use existing action state without adding new routing abstractions or state tracking.

No InputPlumber changes or dependency are introduced. Generic passthrough and PASS-mode behavior remain outside this PR’s scope; this is not a complete fix for issue #549.

Test

Added three Godot regression tests covering:

  • Clearing all active synthetic actions when the Guide-combo marker ends.
  • Ignoring inactive actions.
  • Clearing the screenshot action when RB is released while Guide is held.

All three fail against the previous implementation and pass with these fixes.

Earlier device testing was performed on a ROG Xbox Ally X running
Bazzite-deck Stable F44.20260929. Live verification of the latest release-handling fixes remains to be recorded.

@pastaq

pastaq commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

To mark an issue in github you can use a Fixes: or Closes: tag.

I.e.
Closes: #549

Remove the Guide+A, Guide+X, Guide+Y capture placeholder, and Guide+RB handlers, plus the associated screenshot forwarding path.

This is the wrong direction. As I stated in ShadowBlip/InputPlumber#747 PASS mode needs to always forward all input. OpenGamepadUI is not the only consumer FWIW, but what we really need is a refactor of the overlay mode input manager that returns all input that OpenGamepadUI Doesn't use back to InputPlumber over dbus. That will require detecting when a chord other than guide +b is pressed and matching that to the generic capabilities. I.e. Guide + A -> not used, send Guide + A DOWN chord. On release, send UP chord.

It gets a little complex as IIRC the deamon currently expects to do the full down/up on a timer. We'll need to handle that differently in IP.

Release the synthetic quick-bar action when the Guide-combo marker ends, including when Guide is released before B.

Makes sense, good change.

Replace the default overlay profile's LeftTop/RightTop shortcut aliases with ordinary LeftPaddle1/RightPaddle1 mappings, matching InputPlumber's stock profile.

This will break opening the OpenGamepadUI Quick Bar for devices that have LC/RC and no dedicated second QAM button.

Keep QAM2's existing Guide+East/B mapping, dedicated Steam QAM/Keyboard forwarding, and input capture/menu navigation while the overlay is open.

Correct the Guide+B documentation example and remove the profile's trailing JSON comma.

Also fine.

@pastaq pastaq linked an issue Oct 7, 2026 that may be closed by this pull request
@morheddsb

Copy link
Copy Markdown
Author

@pastaq
While reverting the other changes, I noticed two pre-existing issues in
core/systems/input/overlay_mode_input_manager.gd:

  • Guide-action release handling — _input(), around lines 120–145: The outer
    condition checks for an ogui_guide_action release, then the nested conditions
    check whether the same event releases ogui_north_ov, ogui_south_ov, etc. A
    D-Bus-generated InputEventAction has only one action name, so those nested
    cleanup branches cannot match.

  • Screenshot release handling — _input(), around lines 196–198: Inside the
    event.is_released() branch, the RB condition uses
    is_action_pressed("ogui_rb_ov"). It should check is_action_released(...);
    otherwise an RB-release event won’t clear ogui_sc_ov through this path.

Is this something you would like included?

@pastaq

pastaq commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Sure

@morheddsb

Copy link
Copy Markdown
Author

What changed and why:

  • Removed unreachable release checks that required one event to match two different action names.
  • Replaced them with a small loop that clears active shortcut actions when the Guide-combo marker ends, preventing stuck action state.
  • Fixed the screenshot cleanup to check for RB release, rather than press.
  • Added 3 focused Godot regression tests covering active-action cleanup, ignoring inactive actions, and screenshot release. All fail before the fixes and pass afterward.

Existing shortcuts and controller mappings are preserved. No new production state, APIs, or InputPlumber changes.

@pastaq added the 2 fixes. Also the changes you denied have been removed

@pastaq

pastaq commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

can you update the PR description to match the new commit?
Also, please squash your changes so they are in a single commit.

@morheddsb
morheddsb force-pushed the fix/overlay-guide-shortcut-cleanup branch from 4e3627f to dd75027 Compare October 8, 2026 07:42
@morheddsb

Copy link
Copy Markdown
Author

can you update the PR description to match the new commit? Also, please squash your changes so they are in a single commit.

I think that should be it for those requests, please let me know if something is not as it should be

@pastaq

pastaq commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

I'll need to test this. Thanks.

Please add an Assisted-by: LLM tag to the commit if any LLM's were used.

Clear active synthetic Guide actions when the combo marker ends and correct the RB release predicate. Preserve existing shortcuts and device aliases, correct the Guide+B documentation example, and remove the profile JSON trailing comma.

Add three focused Godot regression tests. No InputPlumber routing or API changes.

Assisted-by: LLM
@morheddsb
morheddsb force-pushed the fix/overlay-guide-shortcut-cleanup branch from dd75027 to 7ad734c Compare October 9, 2026 07:40
@morheddsb

Copy link
Copy Markdown
Author

I added the tag to the commit message, not sure if you meant that kind of tag, or a "real" tag

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.

Guide shortcuts not working on xbox ally x in gamemode

2 participants