From 9384028d4dca6503f8334c4c255be224ccf79769 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 19:15:20 +0000 Subject: [PATCH] Achievement browser: clamp Stats scroll to content, drain queued key repeats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reported bugs on the redesigned achievement screen: 1. On the Stats tab, a held Down ran the scroll offset far past the content (the input layer allows 100 and only the renderer knew the viewport) — the view stopped moving but every Up had to unwind the invisible excess first. render_stats_view now returns the offset it actually rendered (clamped to last-line-at-bottom) and the browser persists it, so the first Up always moves. Content that fits entirely on screen clamps to zero. Same fix reaches the character-select splash browser, whose Stats tab previously could not scroll at all (its Down handler passed the empty category's count of 0). 2. The game loop processed one key event per ~100ms frame in normal mode while terminal auto-repeat delivers ~25-30/s, so a held arrow queued events that kept replaying after release. The event loop now drains everything already queued before each frame, bounded at 32 events per frame so pasted input cannot starve the tick. Verified live (drive-game): a 30-Down burst settles the instant it stops, one Up scrolls immediately from the clamp, and the splash Stats tab scrolls. Regression test pins the renderer clamp; all snapshots byte-identical. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011F9nMko3AYUQBsrCtPjBKp --- src/main.rs | 20 ++++++--- src/main_helpers/overlay.rs | 2 +- src/main_helpers/update.rs | 11 ++++- src/ui/achievement_browser_scene.rs | 66 +++++++++++++++++++++++++++-- src/ui/achievement_details.rs | 7 ++- 5 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index e52c7bb9..6dd6a2ff 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1206,8 +1206,14 @@ fn main() -> io::Result<()> { }; // Drain available events. - // In realtime mode, first poll may block until next frame; subsequent polls - // are non-blocking so we can flush queued input quickly. + // The first poll may block (frame pacing); subsequent polls are + // non-blocking so everything already queued is flushed before the + // next frame. Draining matters in normal mode too: key auto-repeat + // (~25-30/s) outruns the ~10 fps frame rate, and any events left + // queued keep replaying after the key is released (reported on the + // achievement browser). Bounded so a runaway paste can't starve + // the tick loop. + let mut drained_events = 0u32; while event::poll(poll_duration)? { let ev = event::read()?; if let Event::Resize(_, _) = &ev { @@ -1217,9 +1223,7 @@ fn main() -> io::Result<()> { if let Event::Key(key_event) = ev { // Only handle key press events (ignore release/repeat) if key_event.kind != KeyEventKind::Press { - if !realtime_mode { - break; - } + poll_duration = Duration::ZERO; continue; } // Chrono Surge summary: any key dismisses @@ -1442,8 +1446,10 @@ fn main() -> io::Result<()> { InputAction::Continue => {} } } - // Normal mode: process one event per frame. Realtime: drain all. - if !realtime_mode { + // Flush whatever is already queued before drawing the next + // frame (bounded), in both normal and realtime mode. + drained_events += 1; + if drained_events >= 32 { break; } poll_duration = Duration::ZERO; diff --git a/src/main_helpers/overlay.rs b/src/main_helpers/overlay.rs index 1739ab43..53dc1867 100644 --- a/src/main_helpers/overlay.rs +++ b/src/main_helpers/overlay.rs @@ -90,7 +90,7 @@ pub fn draw_game_overlays( extras: &OverlayExtras<'_>, ) { let state: &GameState = ctx.state; - let overlay: &GameOverlay = ctx.overlay; + let overlay: &mut GameOverlay = ctx.overlay; let haven: &haven::Haven = ctx.haven; let haven_ui: &HavenUiState = ctx.haven_ui; let soulforge_ui: &SoulforgeUiState = ctx.soulforge_ui; diff --git a/src/main_helpers/update.rs b/src/main_helpers/update.rs index f5224a8e..9bb83fb8 100644 --- a/src/main_helpers/update.rs +++ b/src/main_helpers/update.rs @@ -1175,7 +1175,16 @@ pub fn show_startup_splash_screen( match key_event.code { KeyCode::Up => achievement_browser.move_up(), KeyCode::Down => { - achievement_browser.move_down(category_achievements.len()) + let count = if achievement_browser.selected_category + == achievements::AchievementCategory::Stats + { + // Stats view scrolls by offset; the renderer + // clamps and persists the real maximum. + 100 + } else { + category_achievements.len() + }; + achievement_browser.move_down(count) } KeyCode::Left | KeyCode::Char(',') | KeyCode::Char('<') => { achievement_browser.prev_category() diff --git a/src/ui/achievement_browser_scene.rs b/src/ui/achievement_browser_scene.rs index 87281996..e7bfe7f8 100644 --- a/src/ui/achievement_browser_scene.rs +++ b/src/ui/achievement_browser_scene.rs @@ -110,7 +110,7 @@ pub fn render_achievement_browser( frame: &mut Frame, area: Rect, achievements: &Achievements, - ui_state: &AchievementBrowserState, + ui_state: &mut AchievementBrowserState, enhancement: &EnhancementProgress, _ctx: &super::responsive::LayoutContext, ) { @@ -151,7 +151,11 @@ pub fn render_achievement_browser( // Content area: stats view or list+detail if ui_state.selected_category == AchievementCategory::Stats { - super::achievement_details::render_stats_view( + // Persist the renderer's clamped scroll: the input layer doesn't + // know the viewport, so without this a held Down runs the offset + // invisibly past the content and [Up] has to unwind the excess + // before anything moves again. + ui_state.selected_index = super::achievement_details::render_stats_view( frame, chunks[2], achievements, @@ -405,13 +409,67 @@ mod tests { } } +#[cfg(test)] +mod stats_scroll_clamp_tests { + use super::*; + use ratatui::{backend::TestBackend, Terminal}; + + fn render_at(ui: &mut AchievementBrowserState, w: u16, h: u16) { + let ach = Achievements::default(); + let enh = crate::enhancement::EnhancementProgress::default(); + let mut t = Terminal::new(TestBackend::new(w, h)).unwrap(); + t.draw(|f| { + let area = f.area(); + let ctx = crate::ui::responsive::LayoutContext::from_size(area.width, area.height); + render_achievement_browser(f, area, &ach, ui, &enh, &ctx); + }) + .unwrap(); + } + + /// Regression (reported 2026-07-14): on the Stats tab a held Down ran + /// the scroll offset far past the content — the view stopped moving + /// but [Up] had to unwind the invisible excess before anything + /// scrolled back. The renderer now persists its clamped offset. + #[test] + fn stats_overscroll_is_clamped_back_by_the_renderer() { + let mut ui = AchievementBrowserState::new(); + ui.selected_category = AchievementCategory::Stats; + + // Way past any real content height (the input layer allows 100). + ui.selected_index = 100; + render_at(&mut ui, 120, 30); + let max_scroll = ui.selected_index; + assert!( + max_scroll < 100, + "the renderer must clamp the persisted offset ({max_scroll})" + ); + + // One Up from the clamp must move immediately — that's the bug. + ui.move_up(); + assert_eq!(ui.selected_index, max_scroll.saturating_sub(1)); + + // The clamp is a fixed point: re-rendering doesn't drift it. + ui.selected_index = max_scroll; + render_at(&mut ui, 120, 30); + assert_eq!(ui.selected_index, max_scroll); + + // On a terminal tall enough to show everything, no scroll at all. + ui.selected_index = 100; + render_at(&mut ui, 120, 200); + assert_eq!( + ui.selected_index, 0, + "content that fits entirely on screen has nothing to scroll" + ); + } +} + #[cfg(test)] mod act_row_render_tests { use super::*; use crate::achievements::Act; use ratatui::{backend::TestBackend, Terminal}; - fn render_rows(ui: &AchievementBrowserState) -> Vec { + fn render_rows(ui: &mut AchievementBrowserState) -> Vec { let ach = Achievements::default(); let enh = crate::enhancement::EnhancementProgress::default(); let mut t = Terminal::new(TestBackend::new(120, 20)).unwrap(); @@ -440,7 +498,7 @@ mod act_row_render_tests { let mut ui = AchievementBrowserState::new(); ui.toggle_act(); assert_eq!(ui.selected_act, Act::ActII); - let rows = render_rows(&ui); + let rows = render_rows(&mut ui); assert!(rows[1].contains("ACT II \u{00b7} The Crossing")); assert!(rows[2].contains("The Voyage")); assert!(rows.iter().any(|r| r.contains("The Burn"))); diff --git a/src/ui/achievement_details.rs b/src/ui/achievement_details.rs index 847eb876..93041573 100644 --- a/src/ui/achievement_details.rs +++ b/src/ui/achievement_details.rs @@ -213,13 +213,17 @@ pub(super) fn render_achievement_detail( } /// Render the stats view (full-width, two columns). +/// Renders the two stats columns and returns the scroll offset actually +/// used — clamped so the last content line stops at the bottom of the +/// viewport. Callers persist the returned value (see the browser scene) +/// so a held Down key can't run the offset invisibly past the content. pub(super) fn render_stats_view( frame: &mut Frame, area: Rect, achievements: &Achievements, enhancement: &EnhancementProgress, scroll_offset: usize, -) { +) -> usize { let block = Block::default() .borders(Borders::ALL) .border_style(Style::default().fg(super::themed_border_color(Color::DarkGray))); @@ -245,6 +249,7 @@ pub(super) fn render_stats_view( render_lines(frame, columns[0], left_lines, scroll); render_lines(frame, columns[2], right_lines, scroll); + scroll } /// Build the left column lines: raw stats with dot-leaders.