From 942b9c2161ea23bdcdf51bc2cddd8e851b30d83a Mon Sep 17 00:00:00 2001 From: funman300 Date: Thu, 25 Jun 2026 10:10:57 -0700 Subject: [PATCH] fix(engine): share Draw-Three waste fan step between renderer and hit-test The waste fan x-offset was computed two ways: the renderer used tableau_col_step * 0.224 while the hit-test hard-coded card_size.x * 0.28. These coincide on desktop (col_step = 1.25*cw) but drift on Android, where tighter column spacing (H_GAP_DIVISOR=32, col_step ~= 1.03*cw) makes the renderer fan at ~0.231*cw. The top fanned waste card's sprite then sits ~14px left of its click target, so dragging the visible top card grabs the card beneath it. Extract waste_fan_step() and tableau_col_step() as the single source for both the renderer (card_plugin::card_positions) and the hit-test (input_plugin::card_position) so they can no longer diverge. Add a regression test that simulates Android-tight spacing and asserts the hit target tracks the renderer. Co-Authored-By: Claude Opus 4.8 (1M context) --- solitaire_engine/src/card_plugin.rs | 58 +++++++++++++++++----------- solitaire_engine/src/input_plugin.rs | 57 +++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/solitaire_engine/src/card_plugin.rs b/solitaire_engine/src/card_plugin.rs index f371b95..2c9bfe5 100644 --- a/solitaire_engine/src/card_plugin.rs +++ b/solitaire_engine/src/card_plugin.rs @@ -63,6 +63,36 @@ pub const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.14; // foundation piles bleeding through when a 2 sits on an Ace. pub const STACK_FAN_FRAC: f32 = 0.025; +/// Per-card horizontal fan step for the Draw-Three waste, in logical pixels. +/// +/// Derived from the actual tableau column spacing (`Tableau2.x − Tableau1.x`) +/// rather than a fixed fraction of card width, so the fan scales with the +/// platform's `H_GAP_DIVISOR` (desktop ≈ 1.25×cw spacing, Android ≈ 1.03×cw). +/// Public so `input_plugin` can hit-test the fanned waste cards at the exact +/// x-offsets the renderer uses; any drift makes a click on the top fanned card +/// land on the card beneath it. +pub fn waste_fan_step(layout: &Layout) -> f32 { + tableau_col_step(layout) * 0.224 +} + +/// Horizontal distance between adjacent tableau columns (`Tableau2.x − +/// Tableau1.x`), in logical pixels. The face-down stock is rendered one column +/// step left of the waste, and the Draw-Three waste fan ([`waste_fan_step`]) is +/// a fraction of it. Public so hit-testing mirrors the renderer exactly. +pub fn tableau_col_step(layout: &Layout) -> f32 { + let t1 = layout + .pile_positions + .get(&KlondikePile::Tableau(Tableau::Tableau1)) + .copied() + .unwrap_or_default(); + let t2 = layout + .pile_positions + .get(&KlondikePile::Tableau(Tableau::Tableau2)) + .copied() + .unwrap_or_default(); + (t2.x - t1.x).abs() +} + /// Font size as a fraction of card width. const FONT_SIZE_FRAC: f32 = 0.28; @@ -908,34 +938,18 @@ fn card_positions(game: &GameState, layout: &Layout) -> Vec<((Card, bool), Vec2, (KlondikePile::Tableau(Tableau::Tableau7), false), ]; - // Compute the Draw-Three waste fan step proportional to the column spacing - // (waste_x − stock_x = card_width + h_gap) rather than a fixed fraction of - // card_width. On desktop (H_GAP_DIVISOR=4) col_step = 1.25×cw and - // 0.224 × 1.25 = 0.28 — identical to the previous constant. On Android - // (H_GAP_DIVISOR=32) col_step ≈ 1.031×cw so fan_step ≈ 0.231×cw, keeping - // the top fanned card's centre within the waste column's own horizontal - // footprint instead of spilling into the adjacent gap. - let tableau_col_step = { - let t1 = layout - .pile_positions - .get(&KlondikePile::Tableau(Tableau::Tableau1)) - .copied() - .unwrap_or_default(); - let t2 = layout - .pile_positions - .get(&KlondikePile::Tableau(Tableau::Tableau2)) - .copied() - .unwrap_or_default(); - (t2.x - t1.x).abs() - }; - let waste_fan_step = tableau_col_step * 0.224; + // Draw-Three waste fan step, proportional to the column spacing so it scales + // with the platform's H_GAP_DIVISOR. Shared with input_plugin's hit-test via + // `waste_fan_step` so the two never drift (a drift puts the top fanned card's + // click target on the card beneath it). + let waste_fan_step = waste_fan_step(layout); for (pile_type, is_stock_area) in piles { let Some(mut base) = layout.pile_positions.get(&pile_type).copied() else { continue; }; if matches!(pile_type, KlondikePile::Stock) && is_stock_area { - base.x -= tableau_col_step; + base.x -= tableau_col_step(layout); } let is_tableau = matches!(pile_type, KlondikePile::Tableau(_)); let is_waste = matches!(pile_type, KlondikePile::Stock) && !is_stock_area; diff --git a/solitaire_engine/src/input_plugin.rs b/solitaire_engine/src/input_plugin.rs index ec8006c..0c9b10d 100644 --- a/solitaire_engine/src/input_plugin.rs +++ b/solitaire_engine/src/input_plugin.rs @@ -35,7 +35,7 @@ use crate::auto_complete_plugin::AutoCompleteState; use crate::card_animation::tuning::AnimationTuning; use crate::card_animation::{CardAnimation, MotionCurve}; use crate::card_plugin::{ - CardEntity, CardEntityIndex, HintHighlight, HintHighlightTimer, STACK_FAN_FRAC, + CardEntity, CardEntityIndex, HintHighlight, HintHighlightTimer, STACK_FAN_FRAC, waste_fan_step, }; use crate::challenge_plugin::CHALLENGE_UNLOCK_LEVEL; use crate::events::{ @@ -1175,12 +1175,15 @@ fn card_position( Vec2::new(base.x, base.y + y_offset) } else if matches!(pile, KlondikePile::Stock) && game.draw_mode() == DrawStockConfig::DrawThree { // In Draw-Three mode the top 3 waste cards are fanned in X to match - // card_plugin::card_positions(). Hit-testing must use the same offsets - // so clicking the visually rightmost (top) card actually registers. + // card_plugin::card_positions(). Hit-testing uses the same `waste_fan_step` + // so clicking the visually rightmost (top) card actually registers — a + // fixed `card_size.x * 0.28` matched the renderer on desktop but drifted + // on Android (tighter column spacing), shifting the top card's hit target + // onto the card beneath it. let pile_len = game.waste_cards().len(); let visible_start = pile_len.saturating_sub(3); let slot = stack_index.saturating_sub(visible_start) as f32; - Vec2::new(base.x + slot * layout.card_size.x * 0.28, base.y) + Vec2::new(base.x + slot * waste_fan_step(layout), base.y) } else { base } @@ -1948,6 +1951,52 @@ mod tests { assert_eq!(result.2, vec![card]); } + #[test] + fn draw_three_waste_hit_test_matches_render_fan_step() { + // Regression: the Draw-Three waste hit-test must use the same fan step as + // the renderer (`card_plugin::waste_fan_step`). The previous hard-coded + // `card_size.x * 0.28` matched the renderer only on desktop (column step = + // 1.25*cw); under tighter Android-style spacing the two drift and the top + // fanned card's click target lands on the card beneath it — so dragging + // the visible top card plays the wrong one. + let mut game = GameState::new(7, DrawStockConfig::DrawThree); + let mut layout = compute_layout(Vec2::new(1280.0, 800.0), 0.0, 0.0, true); + + // Force tight (Android-like) column spacing: ~1.03 * card_width. + let cw = layout.card_size.x; + let base = layout.pile_positions[&KlondikePile::Stock]; + let t1 = layout.pile_positions[&KlondikePile::Tableau(Tableau::Tableau1)]; + layout.pile_positions.insert( + KlondikePile::Tableau(Tableau::Tableau2), + Vec2::new(t1.x + cw * 1.03, t1.y), + ); + + clear_test_piles(&mut game); + let waste = vec![ + Card::new(Deck::Deck1, Suit::Clubs, Rank::Two), + Card::new(Deck::Deck1, Suit::Hearts, Rank::Five), + Card::new(Deck::Deck1, Suit::Spades, Rank::Nine), + Card::new(Deck::Deck1, Suit::Diamonds, Rank::King), + ]; + game.set_test_waste_cards(waste.clone()); + + // visible_start = len-3 = 1, so the top card sits at fan slot 2. + let top_index = waste.len() - 1; + let pos = card_position(&game, &layout, &KlondikePile::Stock, top_index); + + let expected = base.x + 2.0 * waste_fan_step(&layout); + assert!( + (pos.x - expected).abs() < 1e-3, + "hit-test must use the shared waste fan step" + ); + // The old fixed constant would have drifted from the renderer here. + let old = base.x + 2.0 * cw * 0.28; + assert!( + (pos.x - old).abs() > 1.0, + "shared step must differ from the old fixed step under tight spacing" + ); + } + #[test] fn find_draggable_skips_face_down_cards() { let game = GameState::new(42, DrawStockConfig::DrawOne); -- 2.47.3