add option to suppress gamepad bindings for sticks/dpad when in remapped layer#22
Conversation
📝 WalkthroughWalkthroughPer-layer gamepad suppression added: config gains Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as Config Loader
participant Gamepad as Gamepad Poll
participant LayerMgr as Layer Manager
participant Mouse as Mouse/Scroll
participant Emitter as Output Emitter
Config->>Gamepad: load stick/dpad suppress_gamepad flags
Gamepad->>LayerMgr: poll active layers & layer remaps
LayerMgr-->>Gamepad: indicate active layers + suppress flags
Gamepad->>Gamepad: set suppress_left/right_stick_, suppress_dpad_, suppress_triggers_
Gamepad->>Mouse: if not suppressed -> translate stick to mouse/scroll (apply deadzone/sensitivity)
Gamepad->>Emitter: build output state (zero suppressed axes/triggers)
Emitter-->>Gamepad: ack / sync (prev_suppress_* updated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
BANANASJIM
left a comment
There was a problem hiding this comment.
Nice idea — suppressing the base gamepad input when a layer remaps sticks/dpad to mouse/arrows makes a lot of sense to avoid double input.
A few things I noticed:
-
Left stick mouse mode:
process_mouse_stickonly setssuppress_right_stick_, so if a user configuresstick_left = { mode = "mouse", suppress_base_binding = true }, the left stick won't actually be suppressed. Might need to handle both sticks there. -
Naming:
suppress_base_bindingis a bit ambiguous — something likesuppress_gamepadorexclusivemight be clearer about what it does (i.e. "don't emit the raw gamepad axis/dpad when this mode is active"). -
Base config interaction: If someone sets
suppress_base_binding = truein the base config (not inside a layer), the stick/dpad would be permanently zeroed out. Might be worth either documenting that this is layer-only, or adding a warning when it's set outside a layer.
Happy to discuss any of these!
|
Yeah I've noticed I struggle with config option names sometimes lol, I'll swap it to suppress_gamepad. Good catch on the left stick mouse mode. I'll take a look at that and warning and maybe also ignoring it when outside a layer in a couple days. |
|
Sorry for bumping. I noticed that the PR solves the problem except for RT and LT. Also related to the discussion, there is a general problem in wich this config is not possible: Looking at the code right stick is only considered for mouse and left stick is only considered for scroll. |
2ec8b68 to
6c126cb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gamepad.cpp`:
- Around line 511-529: In needs_mouse(), update the layer check so that
left-stick layers that set mode to Mouse are treated the same as Scroll: modify
the condition that currently only tests layer.stick_left->mode ==
StickConfig::Scroll to also accept layer.stick_left->mode == StickConfig::Mouse;
this aligns needs_mouse() with the base logic in get_effective_stick_left() and
the runtime behavior in process_mouse_stick() so an InputDevice is created when
a layer declares stick_left with mode Mouse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9035dd06-b26a-4b5f-b249-412186785568
📒 Files selected for processing (5)
config/config.tomlinclude/vader5/config.hppinclude/vader5/gamepad.hppsrc/config.cppsrc/gamepad.cpp
6c126cb to
3df5a90
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gamepad.cpp (1)
122-135:⚠️ Potential issue | 🟠 Major
needs_mouse()still misses right-stickScroll, which can disable scrolling at runtime.Line 108 and Line 127 only treat right stick
Mouseas requiringInputDevice. Sinceprocess_scroll_stick()now supports right stick scroll, a config using only right-stick scroll may never createinput_, and scroll output won’t work.🔧 Proposed fix
auto needs_mouse(const Config& cfg) -> bool { @@ - if (cfg.right_stick.mode == StickConfig::Mouse) { + if (cfg.right_stick.mode == StickConfig::Mouse || + cfg.right_stick.mode == StickConfig::Scroll) { return true; } @@ - if (layer.stick_right && layer.stick_right->mode == StickConfig::Mouse) { + if (layer.stick_right && + (layer.stick_right->mode == StickConfig::Mouse || + layer.stick_right->mode == StickConfig::Scroll)) { return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gamepad.cpp` around lines 122 - 135, The needs_mouse() loop over cfg.layers misses the case where the right stick uses Scroll, so add a check for layer.stick_right->mode == StickConfig::Scroll (similar to the existing left-stick Scroll check) so configs that rely on right-stick scrolling still create the input_ device; update the needs_mouse() function to test both stick_right Mouse and stick_right Scroll (referencing cfg.layers, needs_mouse(), and StickConfig::Scroll) to ensure process_scroll_stick() will have input_ available at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/gamepad.cpp`:
- Around line 122-135: The needs_mouse() loop over cfg.layers misses the case
where the right stick uses Scroll, so add a check for layer.stick_right->mode ==
StickConfig::Scroll (similar to the existing left-stick Scroll check) so configs
that rely on right-stick scrolling still create the input_ device; update the
needs_mouse() function to test both stick_right Mouse and stick_right Scroll
(referencing cfg.layers, needs_mouse(), and StickConfig::Scroll) to ensure
process_scroll_stick() will have input_ available at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 912b921b-5113-4baa-b826-06268d7effac
📒 Files selected for processing (5)
config/config.tomlinclude/vader5/config.hppinclude/vader5/gamepad.hppsrc/config.cppsrc/gamepad.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- include/vader5/config.hpp
- include/vader5/gamepad.hpp
|
|
lgtm, Thanks for you contribution! |
Adds an optional config value "suppress_base_binding" that can be added to stick_left, stick_right and dpad that suppresses the controllers normal binding while in that layer.
Summary by CodeRabbit