fix: address latest controller review feedback

This commit is contained in:
2026-03-14 21:10:27 -07:00
parent 4f6006f565
commit 81e80b8cf5
4 changed files with 183 additions and 10 deletions

View File

@@ -771,6 +771,46 @@ test('gamepad controller trigger digital mode uses pressed state only', () => {
assert.deepEqual(calls, ['play-audio', 'toggle-mpv-pause']);
});
test('gamepad controller digital trigger bindings ignore analog-only trigger values', () => {
const calls: string[] = [];
const buttons = Array.from({ length: 16 }, () => ({ value: 0, pressed: false, touched: false }));
buttons[6] = { value: 0.9, pressed: false, touched: true };
buttons[7] = { value: 0.9, pressed: false, touched: true };
const controller = createGamepadController({
getGamepads: () => [createGamepad('pad-1', { buttons })],
getConfig: () =>
createControllerConfig({
triggerInputMode: 'digital',
triggerDeadzone: 0.6,
bindings: {
playCurrentAudio: 'rightTrigger',
toggleMpvPause: 'leftTrigger',
},
}),
getKeyboardModeEnabled: () => true,
getLookupWindowOpen: () => true,
getInteractionBlocked: () => false,
toggleKeyboardMode: () => {},
toggleLookup: () => {},
closeLookup: () => {},
moveSelection: () => {},
mineCard: () => {},
quitMpv: () => {},
previousAudio: () => {},
nextAudio: () => {},
playCurrentAudio: () => calls.push('play-audio'),
toggleMpvPause: () => calls.push('toggle-mpv-pause'),
scrollPopup: () => {},
jumpPopup: () => {},
onState: () => {},
});
controller.poll(0);
assert.deepEqual(calls, []);
});
test('gamepad controller maps L3 to mpv pause and keeps unbound audio action inactive', () => {
const calls: string[] = [];
const buttons = Array.from({ length: 16 }, () => ({ value: 0, pressed: false, touched: false }));

View File

@@ -69,6 +69,20 @@ function normalizeRawButtonState(
return Boolean(button.pressed) || button.value >= triggerDeadzone;
}
function resolveTriggerBindingPressed(
button: ControllerButtonState | undefined,
config: ResolvedControllerConfig,
): boolean {
if (!button) return false;
if (config.triggerInputMode === 'digital') {
return Boolean(button.pressed);
}
if (config.triggerInputMode === 'analog') {
return button.value >= config.triggerDeadzone;
}
return normalizeRawButtonState(button, config.triggerDeadzone);
}
function resolveGamepadAxis(gamepad: GamepadLike, axisIndex: number): number {
const value = gamepad.axes[axisIndex];
return typeof value === 'number' && Number.isFinite(value) ? value : 0;
@@ -190,7 +204,13 @@ function resolveDiscreteBindingPressed(
}
if (binding.kind === 'button') {
return normalizeRawButtonState(gamepad.buttons[binding.buttonIndex], config.triggerDeadzone);
const button = gamepad.buttons[binding.buttonIndex];
const isTriggerBinding =
binding.buttonIndex === config.buttonIndices.leftTrigger ||
binding.buttonIndex === config.buttonIndices.rightTrigger;
return isTriggerBinding
? resolveTriggerBindingPressed(button, config)
: normalizeRawButtonState(button, config.triggerDeadzone);
}
const activationThreshold = Math.max(config.stickDeadzone, 0.55);

View File

@@ -142,6 +142,10 @@ function buildContext() {
return { state, dom };
}
function findActionRow(container: ReturnType<typeof createFakeElement>, labelText: string) {
return container.children.find((child) => child.children?.[0]?.textContent === labelText) ?? null;
}
test('controller select modal saves preferred controller from dropdown selection', async () => {
const domHandle = installFakeDom();
const saved: unknown[] = [];
@@ -166,6 +170,7 @@ test('controller select modal saves preferred controller from dropdown selection
syncSettingsModalSubtitleSuppression: () => {},
});
modal.wireDomEvents();
modal.openControllerSelectModal();
state.controllerDeviceSelectedIndex = 1;
@@ -210,6 +215,7 @@ test('controller select modal learn mode captures fresh button input and persist
syncSettingsModalSubtitleSuppression: () => {},
});
modal.wireDomEvents();
modal.openControllerSelectModal();
const firstRow = dom.controllerConfigList.children[1];
@@ -239,3 +245,101 @@ test('controller select modal learn mode captures fresh button input and persist
domHandle.restore();
}
});
test('controller select modal preserves saved axis dpad fallback while relearning', async () => {
const domHandle = installFakeDom();
const saved: unknown[] = [];
Object.defineProperty(globalThis, 'window', {
configurable: true,
value: {
focus: () => {},
electronAPI: {
saveControllerConfig: async (update: unknown) => {
saved.push(update);
},
notifyOverlayModalClosed: () => {},
},
},
});
try {
const { state, dom } = buildContext();
state.controllerConfig!.bindings.leftStickHorizontal = {
kind: 'axis',
axisIndex: 0,
dpadFallback: 'none',
};
const modal = createControllerSelectModal({ state, dom } as never, {
modalStateReader: { isAnyModalOpen: () => false },
syncSettingsModalSubtitleSuppression: () => {},
});
modal.openControllerSelectModal();
const tokenMoveRow = findActionRow(dom.controllerConfigList, 'Token Move');
assert.ok(tokenMoveRow);
const learnButton = tokenMoveRow.children[2].children[0];
learnButton.dispatch('click');
state.controllerRawAxes = [0, 0, 0.85];
modal.updateDevices();
await Promise.resolve();
assert.deepEqual(saved.at(-1), {
bindings: {
leftStickHorizontal: {
kind: 'axis',
axisIndex: 2,
dpadFallback: 'none',
},
},
});
} finally {
domHandle.restore();
}
});
test('controller select modal uses unique picker values for duplicate controller ids', async () => {
const domHandle = installFakeDom();
Object.defineProperty(globalThis, 'window', {
configurable: true,
value: {
focus: () => {},
electronAPI: {
saveControllerConfig: async () => {},
notifyOverlayModalClosed: () => {},
},
},
});
try {
const { state, dom } = buildContext();
state.connectedGamepads = [
{ id: 'same-pad', index: 0, mapping: 'standard', connected: true },
{ id: 'same-pad', index: 1, mapping: 'standard', connected: true },
];
state.activeGamepadId = 'same-pad';
const modal = createControllerSelectModal({ state, dom } as never, {
modalStateReader: { isAnyModalOpen: () => false },
syncSettingsModalSubtitleSuppression: () => {},
});
modal.wireDomEvents();
modal.openControllerSelectModal();
const [firstOption, secondOption] = dom.controllerSelectPicker.children;
assert.notEqual(firstOption.value, secondOption.value);
dom.controllerSelectPicker.value = secondOption.value;
dom.controllerSelectPicker.dispatch('change');
assert.equal(state.controllerDeviceSelectedIndex, 1);
} finally {
domHandle.restore();
}
});

View File

@@ -25,7 +25,7 @@ export function createControllerSelectModal(
syncSettingsModalSubtitleSuppression: () => void;
},
) {
let selectedControllerId: string | null = null;
let selectedControllerKey: string | null = null;
let lastRenderedDevicesKey = '';
let lastRenderedActiveGamepadId: string | null = null;
let lastRenderedPreferredId = '';
@@ -58,6 +58,7 @@ export function createControllerSelectModal(
const definition = getControllerBindingDefinition(actionId);
if (!definition) return;
const config = ctx.state.controllerConfig;
const currentBinding = config?.bindings[actionId];
bindingCapture = createControllerBindingCapture({
triggerDeadzone: config?.triggerDeadzone ?? 0.5,
stickDeadzone: config?.stickDeadzone ?? 0.2,
@@ -68,7 +69,9 @@ export function createControllerSelectModal(
actionId,
bindingType: 'axis',
dpadFallback:
definition.defaultBinding.kind === 'axis' &&
currentBinding?.kind === 'axis' && 'dpadFallback' in currentBinding
? currentBinding.dpadFallback
: definition.defaultBinding.kind === 'axis' &&
'dpadFallback' in definition.defaultBinding
? definition.defaultBinding.dpadFallback
: 'none',
@@ -100,9 +103,13 @@ export function createControllerSelectModal(
.join('||');
}
function getDeviceSelectionKey(device: { id: string; index: number }): string {
return `${device.id}:${device.index}`;
}
function syncSelectedControllerId(): void {
const selected = ctx.state.connectedGamepads[ctx.state.controllerDeviceSelectedIndex];
selectedControllerId = selected?.id ?? null;
selectedControllerKey = selected ? getDeviceSelectionKey(selected) : null;
}
function syncSelectedIndexToCurrentController(): void {
@@ -139,7 +146,7 @@ export function createControllerSelectModal(
const preferredId = ctx.state.controllerConfig?.preferredGamepadId ?? '';
ctx.state.connectedGamepads.forEach((device, index) => {
const option = document.createElement('option');
option.value = device.id;
option.value = getDeviceSelectionKey(device);
option.selected = index === ctx.state.controllerDeviceSelectedIndex;
option.textContent = `${device.id || `Gamepad ${device.index}`} (${[
`#${device.index}`,
@@ -226,9 +233,9 @@ export function createControllerSelectModal(
function updateDevices(): void {
if (!ctx.state.controllerSelectModalOpen) return;
if (selectedControllerId) {
if (selectedControllerKey) {
const preservedIndex = ctx.state.connectedGamepads.findIndex(
(device) => device.id === selectedControllerId,
(device) => getDeviceSelectionKey(device) === selectedControllerKey,
);
if (preservedIndex >= 0) {
ctx.state.controllerDeviceSelectedIndex = preservedIndex;
@@ -353,8 +360,10 @@ export function createControllerSelectModal(
void saveSelectedController();
});
ctx.dom.controllerSelectPicker.addEventListener('change', () => {
const selectedId = ctx.dom.controllerSelectPicker.value;
const selectedIndex = ctx.state.connectedGamepads.findIndex((device) => device.id === selectedId);
const selectedKey = ctx.dom.controllerSelectPicker.value;
const selectedIndex = ctx.state.connectedGamepads.findIndex(
(device) => getDeviceSelectionKey(device) === selectedKey,
);
if (selectedIndex >= 0) {
ctx.state.controllerDeviceSelectedIndex = selectedIndex;
syncSelectedControllerId();