Skip to content

feat(input): drain gamepad hotplug queue + enriched event payloads (#610)#613

Merged
apotema merged 3 commits into
mainfrom
feat/gamepad-drain
Jun 10, 2026
Merged

feat(input): drain gamepad hotplug queue + enriched event payloads (#610)#613
apotema merged 3 commits into
mainfrom
feat/gamepad-drain

Conversation

@apotema

@apotema apotema commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Closes #610.

What

Replaces the engine's fixed-slot gamepad availability scan with a queue drain over the core#18 GamepadEvent contract, and enriches the engine event payloads.

src/game.zig

  • scanInputEvents now drains into a fixed [16]GamepadEvent stack buffer and emits one engine event per drained event (switch on ev.kind).
  • Removed max_tracked_gamepads, the 4-slot cap, and the prev_gamepad_connected[4] edge tracker. Edge detection now belongs to the source.
  • Comptime source selection via @hasDecl(InputImpl, "pollGamepadEvents") — probes the raw backend, not the InputInterface wrapper (whose method always exists and falls back to 0):
    • backend declares it → Self.Input.pollGamepadEvents(&buf)
    • backend does not → core.gamepad_source.pollEvents(&buf) + its init()/deinit() at Game init/deinit
    • never both — no double-source / duplicate events.
  • Source init/deinit gated on uses_os_gamepad_source (!backend_polls_gamepads and gamepad_events_wanted), so they fold away for native-polling backends and event-less games.
  • The whole drain block stays behind the existing comptime gamepad_events_wanted gate → zero-cost when no flow listens (and on the fallback path the OS source is then never polled).

src/root.zig

  • Events.gamepad_connected now carries { id, name, name_len, guid, source_class, type_hint } with a nameSlice() helper. id is kept (first) for backward-compat — flows/hooks reading only .id still compile.
  • name is stored inline ([63:0]u8 + name_len), not as a borrowed []const u8: engine events are copied into event_buffer and dispatched on a later frame, so a slice into the transient drain buffer would dangle.
  • gamepad_disconnected unchanged ({ id }).

test/input_events_test.zig

  • TestInput now declares pollGamepadEvents and injects GamepadEvents instead of toggling availability.
  • Kept connect-once / disconnect / per-slot assertions; added: a multi-event single-drain test (uses slot 7 to prove the 4-slot cap is gone), an enriched-fields propagation test (name/guid/source_class/type_hint), and a fallback test using StubInput (no pollGamepadEvents) that exercises the gamepad_source branch + lifecycle.

Stacking

STACKED on labelle-core#19 (branch feat/gamepad-event-contract, core#18). This PR consumes core.gamepad.GamepadEvent, core.gamepad_source.{init,deinit,pollEvents}, core.GamepadSourceClass, and core.GamepadTypeHint.

build.zig.zon (and scene/build.zig.zon) pin labelle_core to the core#19 git ref — https://github.com/labelle-toolkit/labelle-core/archive/8fcf67641a1d6e021bc305ee62092944456f8f69.tar.gz (commit 8fcf676). This is required because CI clones core's main as a sibling, which does not yet contain the gamepad-event contract, so a ../labelle-core path pin fails to compile on CI. The pin bumps to a release tag once core#19 merges to main — see the TODO in both .zon files. Merge this PR after core#19 lands.

Verification

  • zig build test (Zig 0.16.0, darwin) — green, 28/28 spec tests pass and every per-file test binary runs. Verified the new gamepad tests actually execute (temporarily flipped an assertion → observed the expected failure, then reverted).
  • Both source paths type-check: the backend path via TestInput (declares pollGamepadEvents), the fallback path via StubInput (routes to core.gamepad_source, which is the unsupported stub on darwin → 0 events).

)

Replace the engine's fixed 4-slot availability scan with a queue drain
over the core#18 GamepadEvent contract, and enrich the engine event
payloads.

- game.zig: scanInputEvents now drains `Self.Input.pollGamepadEvents`
  into a fixed `[16]GamepadEvent` stack buffer and emits one engine
  event per drained event. Removes `max_tracked_gamepads`, the 4-slot
  cap, and the `prev_gamepad_connected` per-slot edge tracker — edge
  detection now belongs to the source.
- Comptime source selection via `@hasDecl(InputImpl, "pollGamepadEvents")`
  (probes the raw backend, not the always-present InputInterface
  wrapper). When the backend declares it, use the backend; otherwise
  fall back to `core.gamepad_source.pollEvents` and run its
  `init`/`deinit` at Game init/deinit. Never both — no double-source.
  Lifecycle calls are gated on `uses_os_gamepad_source` so they fold
  away for native-polling backends and event-less games.
- The whole drain block stays behind the existing comptime
  `gamepad_events_wanted` gate, so it's zero-cost when no flow listens
  (and the OS source is never polled).
- root.zig: `gamepad_connected` now carries
  { id, name, name_len, guid, source_class, type_hint } with a
  `nameSlice()` helper; `id` kept first for backward-compat. `name` is
  stored INLINE (not a borrowed slice) because events are copied into
  `event_buffer` and dispatched on a later frame — a slice into the
  transient drain buffer would dangle. `gamepad_disconnected` unchanged
  ({ id }).
- input_events_test.zig: TestInput now declares `pollGamepadEvents` and
  injects GamepadEvents instead of toggling availability. Keeps the
  connect-once / disconnect / per-slot assertions, adds a multi-event
  drain test (slot > 4, proving the cap is gone), an enriched-fields
  propagation test, and a fallback test using StubInput (no
  pollGamepadEvents) to exercise the gamepad_source branch + lifecycle.
@cursor

cursor Bot commented Jun 10, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Behavior change for all games using gamepad connect/disconnect flows (event timing and payload shape); depends on labelle-core#18 types and a pinned core ref until main merges.

Overview
Replaces the engine’s 4-slot gamepad availability diff with a per-tick drain of up to 16 core.GamepadEvents from a single source chosen at comptime: the raw input backend’s pollGamepadEvents when declared, otherwise core.gamepad_source (with init/deinit only when that fallback is needed and flows listen for gamepad events). prev_gamepad_connected is removed; connect/disconnect edges come from the queue, not engine-side polling.

engine__gamepad_connected now carries inline name, optional GUID, source_class, and type_hint (plus nameSlice()); id stays first for callers that only read the slot. gamepad_disconnected is unchanged.

Tests switch from toggling isGamepadAvailable to queuing drained events, add multi-slot and enriched-field coverage, and assert the StubInputgamepad_source fallback compiles and runs lifecycle hooks.

Reviewed by Cursor Bugbot for commit c49c6e5. Bugbot is set up for automated code reviews on this repo. Configure here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the gamepad hotplug system to use an event-driven model instead of polling and diffing slot availability. It introduces compile-time selection between the active input backend's native event polling and a per-OS fallback source, and enriches the gamepad_connected event payload with metadata like name, GUID, source class, and type hint. The feedback recommends defensively capping the name slice length in nameSlice to prevent out-of-bounds panics, adding an assertion in the test helper queue to avoid out-of-bounds writes, and robustly handling partial drains in the test stub's pollGamepadEvents by shifting remaining events instead of clearing them unconditionally.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/root.zig
Comment on lines +327 to +329
pub fn nameSlice(self: *const gamepad_connected) []const u8 {
return self.name[0..self.name_len];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To prevent potential out-of-bounds panics if name_len is corrupted or incorrectly reported by a backend, we should defensively cap the slice length to core.gamepad.NAME_CAPACITY.

        pub fn nameSlice(self: *const gamepad_connected) []const u8 {
            const len = @min(self.name_len, core.gamepad.NAME_CAPACITY);
            return self.name[0..len];
        }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in 70c940cnameSlice() now caps the length with @min(self.name_len, core.gamepad.NAME_CAPACITY). Valid defensive bound since name_len is backend-reported.

Comment on lines +60 to 63
fn queue(ev: core.GamepadEvent) void {
pending_events[pending_len] = ev;
pending_len += 1;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To prevent out-of-bounds writes and provide a clear failure message if too many events are queued in tests, we should assert that pending_len is less than pending_events.len.

    fn queue(ev: core.GamepadEvent) void {
        std.debug.assert(pending_len < pending_events.len);
        pending_events[pending_len] = ev;
        pending_len += 1;
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in 70c940c — added std.debug.assert(pending_len < pending_events.len) to queue().

Comment on lines +93 to 98
pub fn pollGamepadEvents(out: []core.GamepadEvent) usize {
const n = @min(out.len, pending_len);
for (0..n) |i| out[i] = pending_events[i];
pending_len = 0;
return n;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If out.len is smaller than pending_len, the remaining events are currently discarded because pending_len is unconditionally reset to 0. To make the test stub robust against different drain capacities, we should shift any remaining events and decrement pending_len accordingly.

    pub fn pollGamepadEvents(out: []core.GamepadEvent) usize {
        const n = @min(out.len, pending_len);
        for (0..n) |i| out[i] = pending_events[i];
        if (n < pending_len) {
            std.mem.copyForwards(core.GamepadEvent, pending_events[0..(pending_len - n)], pending_events[n..pending_len]);
            pending_len -= n;
        } else {
            pending_len = 0;
        }
        return n;
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Applied in 70c940cpollGamepadEvents() now shifts the undrained tail with copyForwards and decrements pending_len when out.len < pending_len, so it behaves like a real FIFO backend instead of dropping events.

…ew findings

CI: the engine depended on labelle-core via a ../labelle-core path pin,
but CI clones core's main as a sibling, which lacks the gamepad-event
contract (core.gamepad / GamepadEvent), so the ubuntu test build failed
to compile (root has no member 'gamepad'). Pin labelle_core (and
scene/build.zig.zon) to the core#19 git ref 8fcf676
(feat/gamepad-event-contract). Bump to a release tag once core#19 merges.

Review (gemini-code-assist):
- src/root.zig: clamp nameSlice() to NAME_CAPACITY to avoid OOB if a
  backend over-reports name_len.
- test/input_events_test.zig: assert queue() does not overflow the fixed
  pending buffer; drain pollGamepadEvents() as a real FIFO (keep the
  undrained tail when out.len < pending_len) instead of dropping events.
@apotema apotema merged commit 0d794e5 into main Jun 10, 2026
2 of 3 checks passed
apotema added a commit that referenced this pull request Jun 10, 2026
…-lost, resume (#611) (#614)

* feat(input): ControllerManager — player↔controller mapping (#611)

A game-facing layer over the raw core#18 gamepad events: game code
cares about *players*, not hardware slots. Mechanism, not policy — the
engine never hardcodes when a controller becomes a player; it exposes
the pool + assignment API + events, and ships the two common policies
as opt-in helpers.

src/controller_manager.zig — new, allocation-free fixed-capacity state
machine (default 8/8) over the GamepadEvent identity fields:
  - Unassigned pool surfaced via controller_available / controller_removed.
  - Assignment API: assign / unassign / playerFor / controllerFor +
    query/iteration (availableControllers, isPlayerActive, isPlayerWaiting).
  - Player-level events emitted only AFTER the game assigns:
    player_joined / player_controller_lost / player_controller_restored.
  - Debounced-lost (engine-owned): configurable grace window; a transient
    drop that reconnects (same guid) within the window NEVER fires lost —
    no {lost,restored} churn, no pause-dialog flicker on a BT blip.
  - Identity-based resume (engine-owned): same-guid replug rebinds to the
    same player across slot churn (Linux returns a new js* index); on
    backends with no stable key (raylib → guid==null) a documented
    heuristic resumes the most-recently-vacated player.
  - Opt-in policy helpers: autoBindFreeSlots, joinOnButton (NOT default).

src/game.zig — embed the manager (void / zero-size unless a player-level
controller event is wanted). Feed it the SAME drained gamepad events and
drain its output into engine events. Moved the gamepad+controller drain
into scanGamepadEvents, run in the ALWAYS-RUN tick section (before the
pause gate) so an opt-in auto-pause can be lifted by a reconnect seen
while paused. Game-facing forwarders: controllerManager(),
assignController, unassignPlayer, playerForController, controllerForPlayer,
setAutoPauseOnControllerLost (opt-in auto-pause, #465 precedent).

src/root.zig — Events.controller_available / controller_removed /
player_joined / player_controller_lost / player_controller_restored for
flow OnEvent; re-export ControllerManager + Config + ControllerInfo +
ManagerEvent + NO_PLAYER / NO_CONTROLLER.

test/controller_manager_test.zig — 18 tests: pool, assignment/query,
debounce (transient drop does not fire lost), guid resume across slot
churn, raylib heuristic resume, opt-in helpers, and integration through
Game.tick incl. end-to-end auto-pause (lose → pause → same-guid replug
while paused → resume).

Stacked on engine#610 (PR #613, enriched GamepadEvent drain) and
labelle-core#19. Phase 2 of the gamepad epic #609.

* refactor(controller-manager): simplify bindingInfo to return bound_info

The conditional on b.controller_id was redundant: whenever controller_id
is live (!= NO_CONTROLLER) it always equals bound_info.controller_id
(both are written together in assign and restoreBinding, and the only
other writer, onDisconnected, clears controller_id without touching
bound_info). So bound_info already carries the correct live id in every
reachable state. Addresses Gemini review on #614.
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.

feat(input): drain gamepad events via pollGamepadEvents; enrich connect/disconnect payload (Phase 0)

1 participant