diff --git a/solitaire_engine/src/card_plugin.rs b/solitaire_engine/src/card_plugin.rs index a0d0340..6ae7fc2 100644 --- a/solitaire_engine/src/card_plugin.rs +++ b/solitaire_engine/src/card_plugin.rs @@ -742,6 +742,19 @@ fn card_positions<'a>(game: &'a GameState, layout: &Layout) -> Vec<(&'a Card, Ve PileType::Tableau(6), ]; + // 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 waste_fan_step = { + let s = layout.pile_positions.get(&PileType::Stock).copied().unwrap_or_default(); + let w = layout.pile_positions.get(&PileType::Waste).copied().unwrap_or_default(); + (w.x - s.x).abs() * 0.224 + }; + for pile_type in piles { let Some(base) = layout.pile_positions.get(&pile_type) else { continue; @@ -783,7 +796,7 @@ fn card_positions<'a>(game: &'a GameState, layout: &Layout) -> Vec<(&'a Card, Ve // normally — no card is hidden, so the shift is 0. let visible = 3_usize; let hidden = rendered_len.saturating_sub(visible); - slot.saturating_sub(hidden) as f32 * layout.card_size.x * 0.28 + slot.saturating_sub(hidden) as f32 * waste_fan_step } else { 0.0 }; @@ -3380,6 +3393,49 @@ mod tests { } } + /// Regression: on tight layouts (e.g. Android H_GAP_DIVISOR=32) the + /// Draw-Three waste fan must be proportional to column spacing so that no + /// fanned card ever bleeds left into the stock column. + /// + /// The invariant holds structurally (x_offset ≥ 0), but this test pins + /// the formula so a future change that accidentally introduces negative + /// offsets or flips the fan direction is caught immediately. + #[test] + fn waste_cards_do_not_overlap_stock_column_on_portrait() { + use solitaire_core::game_state::DrawMode; + let mut g = GameState::new(42, DrawMode::DrawThree); + for _ in 0..5 { + let _ = g.draw(); + } + + // Android-portrait window. In host tests H_GAP_DIVISOR uses the + // desktop value (4), but the no-overlap invariant must hold on any + // screen size and gap ratio. + let window = Vec2::new(900.0, 2000.0); + let layout = crate::layout::compute_layout(window, 32.0, 110.0, true); + + let stock_x = layout.pile_positions[&PileType::Stock].x; + let stock_right_edge = stock_x + layout.card_size.x / 2.0; + + let waste_ids: std::collections::HashSet = g.piles[&PileType::Waste] + .cards + .iter() + .map(|c| c.id) + .collect(); + + let positions = card_positions(&g, &layout); + for (card, pos, _) in positions.iter().filter(|(c, _, _)| waste_ids.contains(&c.id)) { + let left_edge = pos.x - layout.card_size.x / 2.0; + assert!( + left_edge >= stock_right_edge - 1e-3, + "waste card {} left edge {:.2} overlaps stock right edge {:.2} on portrait window", + card.id, + left_edge, + stock_right_edge, + ); + } + } + #[test] fn waste_pile_draw_one_cards_have_distinct_z() { use solitaire_core::game_state::DrawMode; diff --git a/solitaire_engine/src/layout.rs b/solitaire_engine/src/layout.rs index 3b652d7..f8fcf01 100644 --- a/solitaire_engine/src/layout.rs +++ b/solitaire_engine/src/layout.rs @@ -605,6 +605,74 @@ mod tests { ); } + /// Suspend → resume layout-consistency invariant. + /// + /// If the resume handler resets `SafeAreaInsets` to zero and then the JNI + /// poller re-resolves the same values, `compute_layout` must produce an + /// identical result to the fresh-launch layout. This test also verifies + /// that a layout computed with `safe_area_top = 0` (the brief window while + /// insets haven't re-resolved after resume) differs visibly from the + /// correct layout, confirming that the bug would manifest without the fix. + #[test] + fn suspend_resume_layout_matches_fresh_launch() { + let window = Vec2::new(900.0, 2000.0); + let safe_top = 27.0_f32; + let safe_bottom = 110.0_f32; + + // Fresh-launch layout — insets known from startup. + let fresh = compute_layout(window, safe_top, safe_bottom, true); + + // Layout computed during the brief post-resume window before insets + // re-resolve (safe_area_top temporarily 0). + let wrong = compute_layout(window, 0.0, safe_bottom, true); + + // Verify the "wrong" layout actually differs — the bug would push the + // top card row upward by exactly safe_top pixels. + let fresh_stock_y = fresh.pile_positions[&PileType::Stock].y; + let wrong_stock_y = wrong.pile_positions[&PileType::Stock].y; + // In Bevy's +y-is-up system, adding safe_area_top pushes the stock + // downward (−y direction). So wrong_stock_y > fresh_stock_y by safe_top. + assert!( + (wrong_stock_y - fresh_stock_y - safe_top).abs() < 1e-3, + "wrong layout must displace stock upward by safe_top ({safe_top}): \ + fresh={fresh_stock_y:.2} wrong={wrong_stock_y:.2} delta={:.2}", + wrong_stock_y - fresh_stock_y, + ); + + // After the poller re-resolves correct insets the layout must be + // identical to the fresh-launch layout. + let corrected = compute_layout(window, safe_top, safe_bottom, true); + assert_eq!( + corrected.card_size, fresh.card_size, + "card size must be preserved after resume", + ); + assert!( + (corrected.pile_positions[&PileType::Stock].y - fresh_stock_y).abs() < 1e-3, + "stock y must match fresh launch after resume: \ + corrected={:.2} fresh={fresh_stock_y:.2}", + corrected.pile_positions[&PileType::Stock].y, + ); + assert!( + (corrected.pile_positions[&PileType::Stock].x + - fresh.pile_positions[&PileType::Stock].x) + .abs() + < 1e-3, + "stock x must be unchanged after resume", + ); + // The HUD band top clearance (distance from window top to card top) + // must match as well — this is the quantity directly visible in Bug 2. + let card_top = |layout: &super::Layout| { + layout.pile_positions[&PileType::Stock].y + layout.card_size.y / 2.0 + }; + assert!( + (card_top(&corrected) - card_top(&fresh)).abs() < 1e-3, + "top-of-card must match fresh launch after resume: \ + corrected={:.2} fresh={:.2}", + card_top(&corrected), + card_top(&fresh), + ); + } + /// safe_area_bottom must not affect horizontal positions. #[test] fn safe_area_bottom_does_not_affect_horizontal_layout() { diff --git a/solitaire_engine/src/safe_area.rs b/solitaire_engine/src/safe_area.rs index ce98207..a8d39bb 100644 --- a/solitaire_engine/src/safe_area.rs +++ b/solitaire_engine/src/safe_area.rs @@ -18,6 +18,7 @@ //! changes flow through automatically. use bevy::prelude::*; +use bevy::window::{AppLifecycle, WindowResized}; use crate::ui_modal::ModalScrim; @@ -65,14 +66,25 @@ pub struct SafeAreaInsetsPlugin; impl Plugin for SafeAreaInsetsPlugin { fn build(&self, app: &mut App) { - app.init_resource::() + // Both message types may already be registered by GamePlugin / TablePlugin; + // add_message is idempotent. + app.add_message::() + .add_message::() + .init_resource::() .add_systems( Update, - (apply_safe_area_anchors, apply_safe_area_bottom_anchors, apply_safe_area_to_modal_scrims), + ( + apply_safe_area_anchors, + apply_safe_area_bottom_anchors, + apply_safe_area_to_modal_scrims, + on_app_resumed, + ), ); #[cfg(target_os = "android")] - app.add_systems(Update, android::refresh_insets); + app.init_resource::() + .add_systems(Update, android::refresh_insets) + .add_systems(Update, android::rearm_on_resumed); } } @@ -142,33 +154,73 @@ fn apply_safe_area_to_modal_scrims( } } +/// Emits a synthetic `WindowResized` on `AppLifecycle::WillResume` so that +/// `on_window_resized` (in `table_plugin`) recomputes the board layout with +/// whatever `SafeAreaInsets` are current at that moment. +/// +/// On Android the `android::rearm_on_resumed` system runs in the same frame +/// and resets both `SafeAreaPollTries` and `SafeAreaInsets` to zero, causing +/// `refresh_insets` to re-poll JNI over the next few frames. When it resolves +/// the correct values, `on_safe_area_changed` in `table_plugin` emits a second +/// synthetic `WindowResized` and the layout converges to the right position. +/// +/// On non-Android targets this handler still fires — it ensures that a resume +/// event always refreshes the layout (e.g., after a minimise/restore on +/// desktop) even though insets are always zero. +fn on_app_resumed( + mut lifecycle: MessageReader, + windows: Query<(Entity, &Window)>, + mut resize_events: MessageWriter, +) { + for event in lifecycle.read() { + if !matches!(event, AppLifecycle::WillResume) { + continue; + } + let Some((entity, window)) = windows.iter().next() else { + return; + }; + resize_events.write(WindowResized { + window: entity, + width: window.resolution.width(), + height: window.resolution.height(), + }); + } +} + #[cfg(target_os = "android")] mod android { - use super::SafeAreaInsets; + use super::{AppLifecycle, SafeAreaInsets}; use bevy::prelude::*; + /// Tracks how many frames `refresh_insets` has polled. Stored as a + /// `Resource` (not `Local`) so that `rearm_on_resumed` can reset it to 0 + /// when `AppLifecycle::WillResume` fires, causing the poller to re-query JNI + /// after a background/foreground cycle. + #[derive(Resource, Default)] + pub(super) struct SafeAreaPollTries(pub u32); + /// Polls Android for safe-area insets until we get a non-zero /// reading, then stops. `getRootWindowInsets()` returns `null` (or /// all-zero `Insets`) until the decor view has been laid out, which /// is typically frame 1–3 of a fresh launch. pub(super) fn refresh_insets( mut insets: ResMut, - mut tries: Local, + mut poll: ResMut, ) { // Cap retries so we don't burn CPU forever on edge-to-edge // devices that genuinely report zero insets. const MAX_TRIES: u32 = 120; // ~2 seconds @ 60 fps - if *tries >= MAX_TRIES || insets.is_populated() { + if poll.0 >= MAX_TRIES || insets.is_populated() { return; } - *tries += 1; + poll.0 += 1; match query_insets() { Ok(v) if v.is_populated() => { info!( "safe_area: insets resolved top={} bottom={} left={} right={} (after {} frames)", - v.top, v.bottom, v.left, v.right, *tries + v.top, v.bottom, v.left, v.right, poll.0 ); *insets = v; } @@ -177,13 +229,35 @@ mod android { } Err(e) => { // Don't spam — log once and let polling continue silently. - if *tries == 1 { + if poll.0 == 1 { warn!("safe_area: JNI query failed (will retry): {e}"); } } } } + /// Resets the inset poller and clears cached insets on + /// `AppLifecycle::WillResume` so that `refresh_insets` re-queries JNI in the + /// frames immediately after the app returns to the foreground. + /// + /// Clearing `SafeAreaInsets` to the default (all-zero) fires + /// `on_safe_area_changed` in `table_plugin`, which emits a synthetic + /// `WindowResized`. `on_window_resized` then recomputes the layout; + /// once `refresh_insets` resolves the real values a second synthetic + /// `WindowResized` fires and the layout converges to the correct position. + pub(super) fn rearm_on_resumed( + mut lifecycle: MessageReader, + mut poll: ResMut, + mut insets: ResMut, + ) { + for event in lifecycle.read() { + if matches!(event, AppLifecycle::WillResume) { + poll.0 = 0; + *insets = SafeAreaInsets::default(); + } + } + } + fn query_insets() -> Result { use bevy::android::ANDROID_APP; use jni::{objects::JObject, JavaVM};