Skip to content

refactor(core): address audit findings (bugfixes, dedup, dead-code, comment hygiene) #383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BANANASJIM merged 1 commit into from
Jun 7, 2026
Merged

refactor(core): address audit findings (bugfixes, dedup, dead-code, comment hygiene) #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/core/gyro.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ pub const GyroProcessor = struct {
self.ema_x = self.ema_x * cfg.smoothing + fsrc_x * (1.0 - cfg.smoothing);
self.ema_y = self.ema_y * cfg.smoothing + fsrc_y * (1.0 - cfg.smoothing);

// [3] normalized curve (vader5): normalize [deadzone,max_val]→[0,1], apply pow, sensitivity scale
// [3] normalized curve: normalize [deadzone,max_val]→[0,1], apply pow, sensitivity scale
const scaled_x = applyCurve(self.ema_x, cfg) * cfg.sensitivity_x;
const scaled_y = applyCurve(self.ema_y, cfg) * cfg.sensitivity_y;

// [5] invert
// [4] invert
const final_x = if (cfg.invert_x) -scaled_x else scaled_x;
const final_y = if (cfg.invert_y) -scaled_y else scaled_y;

Expand All @@ -100,7 +100,7 @@ pub const GyroProcessor = struct {
return .{ .rel_x = 0, .rel_y = 0, .joy_x = jx, .joy_y = jy };
}

// [6] sub-pixel accumulation
// [5] sub-pixel accumulation
self.accum_x += final_x;
self.accum_y += final_y;
const dx: i32 = @intFromFloat(@trunc(self.accum_x));
Expand Down Expand Up @@ -158,7 +158,7 @@ fn selectTiltAxis(axis: GyroAxis, ax: i16, ay: i16, az: i16) ?f32 {
.none => null,
.pitch => std.math.atan2(fy, @sqrt(fx * fx + fz * fz)) * radians_to_degrees,
.roll => std.math.atan2(-fx, @sqrt(fy * fy + fz * fz)) * radians_to_degrees,
.yaw => 0.0,
.yaw => null,
};
}

Expand Down Expand Up @@ -278,6 +278,20 @@ test "gyro: joystick tilt response ignores missing accelerometer vector" {
try testing.expectEqual(@as(f32, -8.0), g.ema_y);
}

test "gyro: tilt response yaw axis yields null (no accelerometer representation)" {
var g = GyroProcessor{};
const cfg = GyroConfig{
.mode = "joystick",
.response = .tilt,
.axis_x = .roll,
.axis_y = .yaw,
.smoothing = 0.0,
};
const out = g.processMotion(&cfg, 0, 0, 0, -5735, 0, 8192);
try testing.expect(out.joy_x != null);
try testing.expect(out.joy_y == null);
}

test "gyro: joystick rate response can route roll gyro to X axis" {
var g = GyroProcessor{};
const cfg = GyroConfig{
Expand Down
81 changes: 46 additions & 35 deletions src/core/macro_player.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const ButtonId = state_mod.ButtonId;

/// Analog floor contributed by macros for LT/RT. Mapper merges this into
/// emit_state.lt/rt via @max so a physical press always wins over the macro
/// when stronger (issue #99 — digital BTN_TL2 alone is not seen by SDL/games).
/// when stronger (digital BTN_TL2 alone is not seen by SDL/games).
pub const AxisInjection = struct {
lt: u8 = 0,
rt: u8 = 0,
Expand Down Expand Up @@ -82,7 +82,7 @@ pub const MacroPlayer = struct {
/// of a .gamepad_button target set or clear bits here.
/// pending_tap_release: tap bits ORed by this frame; mapper clears them next frame
/// (same cadence as the remap tap path — see mapper.emitTapEvent).
/// axes: analog LT/RT floor for this frame (issue #99). `.tap` raises the
/// axes: analog LT/RT floor for this frame. `.tap` raises the
/// floor for one frame; `.down`/`.up` flip the player's held-axis state
/// which is re-asserted every frame until cancelled.
pub fn step(
Expand Down Expand Up @@ -187,44 +187,34 @@ pub const MacroPlayer = struct {
/// Called on layer switch / macro cancel. Drops key-up aux events AND clears
/// held gamepad bits from injected_buttons.
pub fn emitPendingReleases(self: *MacroPlayer, aux: *AuxEventList, injected_buttons: *u64) void {
// Walk steps up to step_index, track net held state per name (keys / mouse buttons).
// Gamepad bits are tracked live in self.held_gamepad_buttons and cleared below.
var held: [32]?[]const u8 = [_]?[]const u8{null} ** 32;
var held_len: usize = 0;

for (self.macro.steps[0..self.step_index]) |s| {
switch (s) {
.down => |name| {
if (held_len < held.len) {
held[held_len] = name;
held_len += 1;
}
},
.up => |name| {
for (held[0..held_len], 0..) |h, i| {
if (h) |hn| {
if (std.mem.eql(u8, hn, name)) {
held[i] = held[held_len - 1];
held_len -= 1;
break;
}
}
}
},
.tap => {},
.delay, .pause_for_release => {},
else => unreachable, // .press is eliminated by expandMacroPress at parse time
// Walk executed steps in reverse: a `.down` is net-held unless a later
// `.up` (or an already-emitted release for the same name) covers it.
// Gamepad bits are tracked live in self.held_gamepad_buttons.
const steps = self.macro.steps[0..self.step_index];
var i: usize = steps.len;
while (i > 0) {
i -= 1;
const name = switch (steps[i]) {
.down => |n| n,
else => continue,
};
var covered = false;
for (steps[i + 1 ..]) |later| {
const ln = switch (later) {
.up, .down => |n| n,
else => continue,
};
if (std.mem.eql(u8, ln, name)) {
covered = true;
break;
}
}
}

for (held[0..held_len]) |h| {
const name = h orelse continue;
if (covered) continue;
const target = resolveTargetSafe(name) orelse continue;
switch (target) {
.key => |code| aux.append(.{ .key = .{ .code = code, .pressed = false } }) catch {},
.mouse_button => |code| aux.append(.{ .mouse_button = .{ .code = code, .pressed = false } }) catch {},
.gamepad_button => {},
.disabled, .macro, .chord, .gesture => {},
.gamepad_button, .disabled, .macro, .chord, .gesture => {},
}
}

Expand Down Expand Up @@ -372,6 +362,27 @@ test "macro_player: emitPendingReleases down without up emits release" {
}
}

test "macro_player: emitPendingReleases releases all held keys beyond 32" {
// 40 distinct keys held down, no matching .up — every one must be released.
const names = [_][]const u8{
"KEY_A", "KEY_B", "KEY_C", "KEY_D", "KEY_E", "KEY_F", "KEY_G", "KEY_H",
"KEY_I", "KEY_J", "KEY_K", "KEY_L", "KEY_M", "KEY_N", "KEY_O", "KEY_P",
"KEY_Q", "KEY_R", "KEY_S", "KEY_T", "KEY_U", "KEY_V", "KEY_W", "KEY_X",
"KEY_Y", "KEY_Z", "KEY_0", "KEY_1", "KEY_2", "KEY_3", "KEY_4", "KEY_5",
"KEY_6", "KEY_7", "KEY_8", "KEY_9", "KEY_F1", "KEY_F2", "KEY_F3", "KEY_F4",
};
var steps: [names.len]MacroStep = undefined;
for (names, 0..) |n, i| steps[i] = .{ .down = n };
const m = Macro{ .name = "many", .steps = &steps };
var player = makePlayer(&m);
player.step_index = steps.len;

var aux = AuxEventList{};
var injected: u64 = 0;
player.emitPendingReleases(&aux, &injected);
try testing.expectEqual(@as(usize, names.len), aux.len);
}

test "macro_player: shift_hold — down pause_for_release up" {
const allocator = testing.allocator;
const steps = [_]MacroStep{ .{ .down = "KEY_LEFTSHIFT" }, .pause_for_release, .{ .up = "KEY_LEFTSHIFT" } };
Expand Down
134 changes: 125 additions & 9 deletions src/core/mapper.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,10 @@ pub const Mapper = struct {
pub fn apply(self: *Mapper, delta: GamepadStateDelta, dt_ms: u32, now_ns: i128) !OutputEvents {
// flush pending tap release from previous frame
var aux = AuxEventList{};
if (self.pending_tap_release) |mask| {
self.injected_buttons &= ~mask;
// The pending tap bit is simply not re-injected this frame — injected_buttons
// is zeroed below before output, so the tap release needs no explicit clear.
if (self.pending_tap_release != null) {
self.pending_tap_release = null;
// inject release into emit state at end of this frame
}

self.state.applyDelta(delta);
Expand Down Expand Up @@ -448,6 +448,8 @@ pub const Mapper = struct {
var gyro_joy_x: ?i16 = null;
var gyro_joy_y: ?i16 = null;
var gyro_blend_stick: bool = false;
const left_cfg = self.effectiveStickConfig(.left);
const right_cfg = self.effectiveStickConfig(.right);
{
const gcfg = self.effectiveGyroConfig();
const activate_spec = blk: {
Expand Down Expand Up @@ -490,7 +492,6 @@ pub const Mapper = struct {
self.gyro_proc.reset();
}

const left_cfg = self.effectiveStickConfig(.left);
const left_out = self.stick_left.process(&left_cfg, self.state.ax, self.state.ay, dt_ms);
if (std.mem.eql(u8, left_cfg.mode, "mouse")) {
if (left_out.rel_x != 0) aux.append(.{ .rel = .{ .code = REL_X, .value = left_out.rel_x } }) catch {};
Expand All @@ -500,7 +501,6 @@ pub const Mapper = struct {
if (left_out.hwheel != 0) aux.append(.{ .rel = .{ .code = REL_HWHEEL, .value = left_out.hwheel } }) catch {};
}

const right_cfg = self.effectiveStickConfig(.right);
const right_out = self.stick_right.process(&right_cfg, self.state.rx, self.state.ry, dt_ms);
if (std.mem.eql(u8, right_cfg.mode, "mouse")) {
if (right_out.rel_x != 0) aux.append(.{ .rel = .{ .code = REL_X, .value = right_out.rel_x } }) catch {};
Expand Down Expand Up @@ -640,8 +640,8 @@ pub const Mapper = struct {
// assemble emit state
var emit_state = self.state;
emit_state.buttons = (self.state.buttons & ~self.suppressed_buttons) | self.injected_buttons;
// issue #99: macros driving LT/RT raise the analog axis floor; physical
// input still wins when the user presses harder than the macro.
// macros driving LT/RT raise the analog axis floor; physical input
// still wins when the user presses harder than the macro.
if (macro_axes.lt > emit_state.lt) emit_state.lt = macro_axes.lt;
if (macro_axes.rt > emit_state.rt) emit_state.rt = macro_axes.rt;
emit_state.synthesizeDpadAxes();
Expand Down Expand Up @@ -673,8 +673,6 @@ pub const Mapper = struct {
}

// suppress stick axes when mode != gamepad
const left_cfg = self.effectiveStickConfig(.left);
const right_cfg = self.effectiveStickConfig(.right);
if (!suppress_left_stick_gyro and (left_cfg.suppress_gamepad or !std.mem.eql(u8, left_cfg.mode, "gamepad"))) {
emit_state.ax = 0;
emit_state.ay = 0;
Expand Down Expand Up @@ -1383,6 +1381,63 @@ test "mapper: base remap key: source -> KEY_F13 aux event" {
}
}

test "mapper: gesture tap remap emits tap key through apply" {
const allocator = testing.allocator;
const parsed = try makeMapping(
\\[remap]
\\A = { tap = "KEY_X", hold = "KEY_Y" }
, allocator);
defer parsed.deinit();

var m = try makeMapper(&parsed.value, allocator);
defer m.deinit();

const a_mask = buttonBit("A");
// Press: no emit yet (tap deferred until release decides leg).
_ = try m.apply(.{ .buttons = a_mask }, 16, 0);
// Release before hold deadline: tap fires.
const ev = try m.apply(.{ .buttons = 0 }, 16, 10 * std.time.ns_per_ms);
try testing.expectEqual(@as(usize, 1), ev.aux.len);
switch (ev.aux.get(0)) {
.key => |k| {
try testing.expectEqual(@as(u16, 45), k.code); // KEY_X
try testing.expect(k.pressed);
},
else => return error.WrongEventType,
}
}

test "mapper: gesture hold gamepad bit persists then clears on release" {
const allocator = testing.allocator;
const parsed = try makeMapping(
\\[remap]
\\A = { tap = "X", hold = "RB", hold_ms = 100 }
, allocator);
defer parsed.deinit();

var m = try makeMapper(&parsed.value, allocator);
defer m.deinit();

const a_mask = buttonBit("A");
const rb_mask = buttonBit("RB");

// Press arms the hold timer.
_ = try m.apply(.{ .buttons = a_mask }, 16, 0);

// Hold deadline fires while button still held -> gamepad press staged.
_ = m.onMacroTimerExpired(100 * std.time.ns_per_ms + 1);
try testing.expect((m.gesture_held_gamepad & rb_mask) != 0);

// Next frame re-asserts the held bit into output.
const held_ev = try m.apply(.{ .buttons = a_mask }, 16, 110 * std.time.ns_per_ms);
try testing.expect((held_ev.gamepad.buttons & rb_mask) != 0);

// Release clears the held bit; output no longer carries RB.
const rel_ev = try m.apply(.{ .buttons = 0 }, 16, 120 * std.time.ns_per_ms);
try testing.expectEqual(@as(u64, 0), m.gesture_held_gamepad & rb_mask);
try testing.expectEqual(@as(u64, 0), rel_ev.gamepad.buttons & rb_mask);
}

test "mapper: releaseHeldAux releases old trigger-threshold aux down" {
const allocator = testing.allocator;
const old_parsed = try makeMapping(
Expand Down Expand Up @@ -1693,6 +1748,67 @@ test "mapper: hold_toggle timer transition resets processors" {
try testing.expectEqual(@as(f32, 0), m.stick_right.scroll_accum);
}

test "mapper: pause_for_release macro drained on layer deactivation" {
const allocator = testing.allocator;
const parsed = try makeMapping(
\\[[layer]]
\\name = "fn"
\\trigger = "Select"
\\activation = "toggle"
\\
\\[remap]
\\M1 = "macro:shift_hold"
\\
\\[[macro]]
\\name = "shift_hold"
\\steps = [
\\ { down = "KEY_LEFTSHIFT" },
\\ "pause_for_release",
\\ { up = "KEY_LEFTSHIFT" },
\\]
, allocator);
defer parsed.deinit();

var m = try makeMapper(&parsed.value, allocator);
defer m.deinit();

const sel_mask = buttonBit("Select");
const m1_mask = buttonBit("M1");

// Toggle the layer on (rising then falling edge on Select).
_ = try m.apply(.{ .buttons = sel_mask }, 16, 0);
_ = try m.apply(.{ .buttons = 0 }, 16, 0);
try testing.expect(m.layer.getActive(m.config.layer.?) != null);

// Trigger the macro: down LEFTSHIFT fires, pause_for_release halts.
const ev_press = try m.apply(.{ .buttons = m1_mask }, 16, 0);
try testing.expectEqual(@as(usize, 1), m.active_macros.items.len);
try testing.expect(m.active_macros.items[0].waiting_for_release);
var saw_press = false;
for (ev_press.aux.slice()) |e| switch (e) {
.key => |k| if (k.code == 42 and k.pressed) {
saw_press = true;
},
else => {},
};
try testing.expect(saw_press);

// Toggle the layer off: deactivation drains the paused macro to completion,
// emitting the up=LEFTSHIFT release, and clears active_macros.
_ = try m.apply(.{ .buttons = m1_mask | sel_mask }, 16, 0);
const ev_off = try m.apply(.{ .buttons = m1_mask }, 16, 0);
try testing.expect(m.layer.getActive(m.config.layer.?) == null);
try testing.expectEqual(@as(usize, 0), m.active_macros.items.len);
var saw_release = false;
for (ev_off.aux.slice()) |e| switch (e) {
.key => |k| if (k.code == 42 and !k.pressed) {
saw_release = true;
},
else => {},
};
try testing.expect(saw_release);
}

test "mapper: hold_toggle pending preserves macros but sticky transition cancels them" {
const allocator = testing.allocator;
const parsed = try makeMapping(
Expand Down
Loading