Skip to content

input/mnk: emit keystrokes so XamInputGetKeystroke returns input#311

Open
Zorkats wants to merge 4 commits intorexglue:mainfrom
Zorkats:main
Open

input/mnk: emit keystrokes so XamInputGetKeystroke returns input#311
Zorkats wants to merge 4 commits intorexglue:mainfrom
Zorkats:main

Conversation

@Zorkats
Copy link
Copy Markdown

@Zorkats Zorkats commented Apr 29, 2026

Closes #310

Input

  • Emit keystrokes from MnkInputDriver so XamInputGetKeystroke / XamInputGetKeystrokeEx return input from keyboard and mouse. GetState now diffs the freshly computed buttons, triggers, and left-stick direction against tracked previous-frame state and enqueues the matching VirtualKey::kXInputPad* code per transition. Right stick (mouse-driven and continuous) is intentionally skipped to avoid flooding the queue; Guide button is omitted because it has no VK_PAD code in the standard XInput keystroke spec.

Fixes

  • Populate the MnkInputDriver keystroke queue. EnqueueKeystroke was defined but never called from any event handler, leaving keystroke_queue_ permanently empty and causing XamInputGetKeystroke / XamInputGetKeystrokeEx to always return X_ERROR_EMPTY whenever MnK was the input source. The bug was silent on most gameplay (state polling via XamInputGetState worked correctly, so WASD movement, intro skipping, and in-game button input all functioned) but broke any title screen or menu that consumed edge-triggered input — "Press Start" prompts appeared unresponsive even though intros could be skipped fine. GetState now invokes a new EmitKeystrokes(buttons, lt, rt, lx, ly) after computing the gamepad state, inside the existing state_mutex_ lock. The method table-drives the button bit → kXInputPad* mapping (D-pad, Start, Back, A/B/X/Y, both shoulders, both thumb-presses), thresholds triggers digitally (any nonzero deflection emits kXInputPadLTrigger / kXInputPadRTrigger, sufficient because MnK only ever produces 0 or 0xFF), and classifies the left stick into a 9-state direction enum where diagonals win over cardinals when both axes deflect. OnLostFocus resets the new prev-state members alongside the existing key_down_ reset so resumed focus does not replay stale keystrokes. Tested against TiP-Recomp on 0.7.4 and forward-applied to 0.7.6 (mnk driver files byte-identical between those versions).

@tomcl7 tomcl7 self-assigned this Apr 30, 2026
@tomcl7 tomcl7 self-requested a review April 30, 2026 19:49
Copy link
Copy Markdown
Member

@tomcl7 tomcl7 left a comment

Choose a reason for hiding this comment

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

I did confirm that EnqueueKeystroke had no callers, but this fix only covers part of what the issue describes though.

  • No repeat events. EmitKeystrokes only emits on rising and falling edges, but native XInputGetKeystroke fires XINPUT_KEYSTROKE_REPEAT after an initial delay and at a repeat rate. Issue #310 calls out "360 dashboard style menus", which need repeats for hold to scroll. After this PR a "Press Start" prompt advances, but a hold to scroll menu ticks once per press from MnK and scrolls continuously from a real pad.
  • Edge detection at GetState time drops fast taps. A press and release that begins and ends between two GetState calls leaves the buttons field at 0 in both samples and emits nothing. The transitions are visible in OnKeyDown and OnKeyUp, which is where edge detection wants to live.
  • EmitKeystrokes is only called from GetState. A title that polls only XamInputGetKeystroke, or polls keystrokes much more often than state, won't see anything from MnK.
  • OnLostFocus resets prev_* but doesn't drain keystroke_queue_. Before this PR the queue was always empty so this was moot. Now anything queued just before focus loss sits there and gets delivered after focus returns.
  • The right stick comment frames the alternative as impossible ("would flood the queue"). It's a tradeoff. Worth saying so, since this silently breaks any title that uses right stick keystrokes for menu navigation.

@Zorkats Zorkats requested a review from tomcl7 May 1, 2026 20:00
@Zorkats
Copy link
Copy Markdown
Author

Zorkats commented May 1, 2026

I have added your feedback to the fork. Left it commented with clear explanations in case you have any doubts. Tested it and Built it, still works with TiP-Recomp.

Copy link
Copy Markdown
Member

@tomcl7 tomcl7 left a comment

Choose a reason for hiding this comment

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

The bug at #310 is real, but the PR body describes a different fix than what's in the diff. It says GetState "diffs the freshly computed buttons, triggers, and left-stick direction against tracked previous-frame state" and invokes "a new EmitKeystrokes(buttons, lt, rt, lx, ly)." There is no such function in the diff. KEYDOWN/KEYUP edges actually come from OnKeyDown / OnKeyUp / OnMouseDown / OnMouseUp calling EmitButtonChange directly

void MnkInputDriver::OnKeyDown(rex::ui::KeyEvent& e) {
if (!IsEnabled() || !has_focus_)
return;
std::lock_guard lock(state_mutex_);
VirtualKey vk = e.virtual_key();
uint16_t idx = static_cast<uint16_t>(vk);
// OS auto-repeat fires OnKeyDown repeatedly while the key is held; only the
// first transition (was-up -> now-down) should emit a KEYDOWN keystroke.
// XINPUT_KEYSTROKE_REPEAT is generated by TickRepeats() on its own schedule.
bool was_down = (idx < 256) && key_down_[idx];
SetKeyState(idx, true);
if (!was_down) {
EmitButtonChange(vk, true);
RecomputeLstickDir();
}
}
and GetState only ticks repeats and the opt-in right stick
packet_number_++;
// Tick keystroke repeats so titles polling state see XINPUT_KEYSTROKE_REPEAT
// events for held buttons. The actual KEYDOWN/KEYUP edges are emitted in the
// event handlers (OnKeyDown/Up/MouseDown/Up) so fast taps that happen between
// GetState calls are not dropped.
TickRepeats();
// Right-stick keystrokes are opt-in (mnk_emit_rstick_keystrokes); see comment
// on the cvar definition for the tradeoff.
EnqueueRStickIfChanged(clamp16(rx), clamp16(ry));
The trigger description is wrong too: there's no threshold anywhere, triggers route through the same keybind table as A/B/X/Y inside EmitButtonChange
void MnkInputDriver::EmitButtonChange(VirtualKey key_vk, bool down) {
auto try_match = [this, key_vk, down](const std::string& cvar_val, PadIdx idx,
VirtualKey vk_pad) {
if (rex::ui::ParseVirtualKey(cvar_val) == key_vk) {
HandleEdge(idx, static_cast<uint16_t>(vk_pad), down);
}
};
try_match(REXCVAR_GET(keybind_a), kPadIdxA, VirtualKey::kXInputPadA);
try_match(REXCVAR_GET(keybind_b), kPadIdxB, VirtualKey::kXInputPadB);
try_match(REXCVAR_GET(keybind_x), kPadIdxX, VirtualKey::kXInputPadX);
try_match(REXCVAR_GET(keybind_y), kPadIdxY, VirtualKey::kXInputPadY);
try_match(REXCVAR_GET(keybind_left_shoulder), kPadIdxLB, VirtualKey::kXInputPadLShoulder);
try_match(REXCVAR_GET(keybind_right_shoulder), kPadIdxRB, VirtualKey::kXInputPadRShoulder);
try_match(REXCVAR_GET(keybind_start), kPadIdxStart, VirtualKey::kXInputPadStart);
try_match(REXCVAR_GET(keybind_back), kPadIdxBack, VirtualKey::kXInputPadBack);
try_match(REXCVAR_GET(keybind_lstick_press), kPadIdxL3, VirtualKey::kXInputPadLThumbPress);
try_match(REXCVAR_GET(keybind_rstick_press), kPadIdxR3, VirtualKey::kXInputPadRThumbPress);
try_match(REXCVAR_GET(keybind_dpad_up), kPadIdxDU, VirtualKey::kXInputPadDpadUp);
try_match(REXCVAR_GET(keybind_dpad_down), kPadIdxDD, VirtualKey::kXInputPadDpadDown);
try_match(REXCVAR_GET(keybind_dpad_left), kPadIdxDL, VirtualKey::kXInputPadDpadLeft);
try_match(REXCVAR_GET(keybind_dpad_right), kPadIdxDR, VirtualKey::kXInputPadDpadRight);
try_match(REXCVAR_GET(keybind_left_trigger), kPadIdxLT, VirtualKey::kXInputPadLTrigger);
try_match(REXCVAR_GET(keybind_right_trigger), kPadIdxRT, VirtualKey::kXInputPadRTrigger);
// Guide is intentionally omitted: standard XInput keystroke spec has no
// VK_PAD_GUIDE; the bit lives only in XInputGetStateEx's state.
}

Rewrite the PR description to match what the code actually does and I'll take another look.

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.

[Bug]: MnK driver: XamInputGetKeystroke always returns X_ERROR_EMPTY

2 participants