Skip to content

fix(av-sync): make the A/V offset slider actually settable + un-clobberable #1046

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

Open
ahonnecke wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(av-sync): make the A/V offset slider actually settable + un-clobberable #1046

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
11 changes: 10 additions & 1 deletion lib/routers/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ def save_settings(data: dict):
except (TypeError, ValueError, OverflowError):
return {"error": "av_offset_ms must be a number between -1000 and 1000"}

if "av_offset_manual" in data:
# Whether av_offset_ms was hand-set by the user. When true the client
# protects the value from auto-calibrate overwrites across reloads.
raw = data["av_offset_manual"]
if raw is not None:
if not isinstance(raw, bool):
return {"error": "av_offset_manual must be a boolean"}
updates["av_offset_manual"] = raw

# fee[dB]ack v0.3.0 gameplay settings (tabbed settings page). null is a
# no-op per the merge contract; bad shapes return a structured error
# rather than 500. countdown_before_song is consumed by the song-start
Expand Down Expand Up @@ -362,7 +371,7 @@ def save_settings(data: dict):
# known set means a malformed or hostile body can't wipe unrelated config.
_RESETTABLE_SETTINGS_KEYS = frozenset({
"default_arrangement", "demucs_server_url", "master_difficulty",
"av_offset_ms", "countdown_before_song", "miss_penalty", "fail_behavior",
"av_offset_ms", "av_offset_manual", "countdown_before_song", "miss_penalty", "fail_behavior",
"reference_pitch", "instrument", "string_count", "tuning", "pathway",
"instrument_profiles", "active_instrument_profile",
"achievements_enabled", "use_amp_sims",
Expand Down
35 changes: 28 additions & 7 deletions static/js/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ export async function loadSettings() {
// render clock, the Settings slider, the HUD readout, and the
// module variable all pick it up consistently. Pass skipPersist
// so we don't echo the loaded value back to the server.
setAvOffsetMs(Number(data.av_offset_ms) || 0, /* skipPersist */ true);
setAvOffsetMs(Number(data.av_offset_ms) || 0, /* skipPersist */ true, 'load');
// Restore the hand-set protection so a value dialed in a prior session
// keeps auto-calibrate from overwriting it after reload.
_avUserOverride = !!data.av_offset_manual;
// Arrangement naming mode is localStorage-only (client preference).
const namingModeEl = document.getElementById('arrangement-naming-mode');
if (namingModeEl) namingModeEl.value = _getArrangementNamingMode();
Expand Down Expand Up @@ -585,13 +588,23 @@ export function handleSliderInput(el) {
// setAvOffsetMs without saving (skipPersist=true) to avoid an
// echo-back round-trip.
export let _avOffsetMs = 0;
// Sticky once the user dials A/V in by hand this session. An auto-calibrate
// sweep (note_detect) or any other programmatic caller must NOT overwrite a
// hand-set value — that's the "I set it and it's wrong again" bug. Persisted as
// av_offset_manual so it survives a reload too.
export let _avUserOverride = false;

export let _avSaveDebounce = null;

export function setAvOffsetMs(ms, skipPersist) {
// Clamp to the same bounds the Settings/player-bar sliders enforce
// (-1000..1000 ms). Defends against bad values from /api/settings
// landing as `value` on <input type=range>.
// `source`: 'manual' = a user control (marks the override); 'load' = seeding the
// saved value (no override); anything else (undefined — the plugin's
// window.setAvOffsetMs auto-calibrate call) is IGNORED while an override holds.
export function setAvOffsetMs(ms, skipPersist, source) {
if (source !== 'manual' && source !== 'load' && _avUserOverride) return;
if (source === 'manual') _avUserOverride = true;
// Clamp to ±1000 (the exact-entry number box may exceed the ±300 slider
// range for edge cases); the sliders themselves are ±300 for real
// resolution. Defends against bad values from /api/settings.
const n = Number(ms);
_avOffsetMs = Math.max(-1000, Math.min(1000, Number.isFinite(n) ? n : 0));
// Drive the highway's render-time shift. getTime() still returns
Expand All @@ -613,6 +626,11 @@ export function setAvOffsetMs(ms, skipPersist) {
playerAvSlider.value = _avOffsetMs;
handleSliderInput(playerAvSlider);
}
// Exact-entry number box — don't fight the user while they're typing in it.
const playerAvNum = document.getElementById('player-av-offset-num');
if (playerAvNum && document.activeElement !== playerAvNum) {
playerAvNum.value = Math.round(_avOffsetMs);
}
const playerAvLabel = document.getElementById('player-av-offset-label');
if (playerAvLabel) {
const rounded = Math.round(_avOffsetMs);
Expand All @@ -638,7 +656,9 @@ export function _persistAvOffset() {
await fetch('/api/settings', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ av_offset_ms: _avOffsetMs }),
// Persist whether this value was hand-set so a reload keeps
// protecting it from auto-calibrate (see setAvOffsetMs / load).
body: JSON.stringify({ av_offset_ms: _avOffsetMs, av_offset_manual: _avUserOverride }),
});
} catch (e) {
console.warn('A/V offset save failed:', e);
Expand All @@ -647,7 +667,8 @@ export function _persistAvOffset() {
}

export function nudgeAvOffsetMs(delta) {
setAvOffsetMs(Math.max(-1000, Math.min(1000, _avOffsetMs + delta)));
// Keyboard [ / ] is a hand adjustment → mark the manual override.
setAvOffsetMs(Math.max(-1000, Math.min(1000, _avOffsetMs + delta)), false, 'manual');
}

export async function saveSettings() {
Expand Down
11 changes: 7 additions & 4 deletions static/v3/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ <h3>Gameplay Settings</h3>
<div class="fb-srow-desc">Positive = audio plays ahead of visual notes; raise to catch the highway up. Adjust live with the [ and ] keys (Shift for ±50 ms). Auto-saves.</div>
</div>
</div>
<input type="range" id="setting-av-offset" min="-1000" max="1000" step="1" value="0"
oninput="setAvOffsetMs(this.value)" class="fb-srow-wide slider-input">
<input type="range" id="setting-av-offset" min="-300" max="300" step="1" value="0"
oninput="setAvOffsetMs(this.value, false, 'manual')" class="fb-srow-wide slider-input">
</div>
<!-- Note highway speed (master difficulty) -->
<div class="fb-srow fb-srow-stack">
Expand Down Expand Up @@ -1208,10 +1208,13 @@ <h3>Gameplay Settings</h3>
<div class="v3-pop-row">
<span class="v3-pop-label" id="player-av-offset-slider-label">A/V sync (ms)</span>
<span class="flex items-center gap-2">
<input type="range" id="player-av-offset-slider" min="-1000" max="1000" value="0" step="1" oninput="setAvOffsetMs(this.value)" class="accent-accent slider-input" title="A/V sync offset (ms) — positive = audio plays ahead of visuals. [ and ] adjust ±10 ms (Shift = ±50). Double-click to reset." ondblclick="setAvOffsetMs(0)" aria-labelledby="player-av-offset-slider-label">
<span id="player-av-offset-label" class="v3-pop-val tabular-nums">+0ms</span>
<input type="range" id="player-av-offset-slider" min="-300" max="300" value="0" step="1" oninput="setAvOffsetMs(this.value, false, 'manual')" class="accent-accent slider-input" title="A/V sync offset (ms). Higher = notes reach the strike line SOONER (visuals sped up); lower = later. [ and ] adjust ±10 ms (Shift = ±50). Double-click to reset." ondblclick="setAvOffsetMs(0, false, 'manual')" aria-labelledby="player-av-offset-slider-label">
<input type="number" id="player-av-offset-num" min="-1000" max="1000" step="1" value="0" onchange="setAvOffsetMs(this.value, false, 'manual')" class="v3-pop-val tabular-nums" style="width:4.4em" title="Type an exact A/V offset in ms (arrows step ±1)" aria-labelledby="player-av-offset-slider-label">
</span>
</div>
<div class="v3-pop-row" style="margin-top:-2px">
<span class="v3-pop-label" style="opacity:.6;font-size:.7rem">↑ higher = notes sooner (visuals faster) · ↓ lower = later · type an exact value on the right</span>
</div>
<div class="v3-pop-row">
<span class="v3-pop-label">Loop</span>
<span class="flex items-center gap-1 flex-wrap">
Expand Down