From 18af49c0f3fff72b26cf8cdb8469e3bb26016a12 Mon Sep 17 00:00:00 2001 From: funman300 Date: Fri, 26 Jun 2026 13:11:00 -0700 Subject: [PATCH] fix(engine): fill tableau fan to viewport on all aspect ratios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a near-square viewport (e.g. an unfolded Galaxy Fold) a fresh deal left the bottom ~40% of the screen empty: the dynamic fan (update_tableau_fan_frac) measured only face-up column depth and returned early at a fresh deal (face-up depth 1), so it never spread the tableau, and the deep face-down stacks were ignored. The cold-start deal also never fired StateChangedEvent, and on Android the safe-area-inset resize (frames 1-3) reset the fan to compute_layout's sparse worst-case value, so even the post-move fill was wiped. Move the fill into layout::apply_dynamic_tableau_fan, driven by each column's TOTAL weighted depth (face-down cards count, scaled by the face-down/face-up step ratio) so the deepest column fills the available height. Run it in three places so every path stays filled: PostStartup (cold-start deal), on StateChangedEvent (moves), and inside on_window_resized after compute_layout (safe-area resize + fold/unfold). MAX_DYNAMIC_FAN_FRAC caps the spread so a near-empty column keeps readable overlap; TABLEAU_FAN_FRAC floors it. Deeper columns drive the fraction down so everything still fits — no overflow. card_position/card_positions read the same fractions, so hit-testing stays in sync. Adds regression tests: cold-start deal fills the fan, and a resize re-fills it. Co-Authored-By: Claude Opus 4.8 (1M context) --- solitaire_engine/src/card_plugin.rs | 207 +++++++++++++++++++-------- solitaire_engine/src/layout.rs | 80 +++++++++++ solitaire_engine/src/table_plugin.rs | 18 ++- 3 files changed, 244 insertions(+), 61 deletions(-) diff --git a/solitaire_engine/src/card_plugin.rs b/solitaire_engine/src/card_plugin.rs index c883c52..3825831 100644 --- a/solitaire_engine/src/card_plugin.rs +++ b/solitaire_engine/src/card_plugin.rs @@ -25,7 +25,7 @@ use crate::card_animation::CardAnimation; use crate::events::{CardFaceRevealedEvent, CardFlippedEvent, StateChangedEvent}; use crate::font_plugin::FontResource; use crate::game_plugin::GameMutation; -use crate::layout::{Layout, LayoutResource, LayoutSystem, TABLEAU_FAN_FRAC}; +use crate::layout::{Layout, LayoutResource, LayoutSystem}; use crate::pause_plugin::PausedResource; use crate::platform::USE_TOUCH_UI_LAYOUT; use crate::resources::{DragState, GameStateResource}; @@ -39,7 +39,7 @@ use crate::ui_theme::{ }; /// Per-card vertical step for face-down tableau cards, as a fraction of -/// card height. Smaller than [`TABLEAU_FAN_FRAC`] because face-down cards +/// card height. Smaller than [`crate::layout::TABLEAU_FAN_FRAC`] because face-down cards /// don't need their full body shown — only the back-pattern strip is /// visible. Public so `input_plugin` can mirror the exact sprite layout /// when hit-testing tableau columns; any drift between this and the @@ -525,7 +525,13 @@ impl Plugin for CardPlugin { .add_systems(Startup, load_card_images) .add_systems( PostStartup, - (sync_cards_startup, update_stock_empty_indicator_startup), + ( + // Fill the tableau fan before the first render so the + // cold-start deal already uses the full viewport height. + fill_tableau_fan_on_startup.before(sync_cards_startup), + sync_cards_startup, + update_stock_empty_indicator_startup, + ), ) .add_systems( Update, @@ -2473,15 +2479,21 @@ fn resize_android_corner_labels( } } -/// Adjusts `LayoutResource.tableau_fan_frac` to match the current maximum -/// face-up column depth. Runs after every `StateChangedEvent` so the fan -/// expands as the player reveals cards while staying within the window. +/// Adjusts `LayoutResource.tableau_fan_frac` (and the face-down companion) so +/// the deepest tableau column fills the available vertical space at every stage +/// of play. Runs after every `StateChangedEvent`. /// -/// On fresh deal (max face-up depth = 1) the function returns early, leaving -/// both fracs at the window-size-adaptive values that `compute_layout` already -/// computed for the current viewport. Previously it overwrote the adaptive -/// value with the desktop minimum (0.25) — the wrong behaviour on portrait -/// phones where the adaptive value is much larger. +/// Depth is measured across *all* cards in a column, weighting each face-down +/// card by the fixed face-down/face-up step ratio. Counting the face-down +/// portion — not just the face-up tail — is what fills the lower screen on a +/// fresh deal (the deepest column is then six face-down cards under one face-up +/// one): the earlier face-up-only depth was 1, so the fan never spread and the +/// bottom half of a near-square viewport (e.g. an unfolded foldable) sat empty. +/// +/// Deeper columns drive the fraction down so everything still fits the window; +/// [`crate::layout::TABLEAU_FAN_FRAC`] floors it to the desktop feel and +/// [`MAX_DYNAMIC_FAN_FRAC`] caps it so a near-empty column doesn't fling its few +/// cards far apart. fn update_tableau_fan_frac( mut events: MessageReader, game: Option>, @@ -2496,59 +2508,35 @@ fn update_tableau_fan_frac( let Some(layout) = layout.as_mut() else { return; }; - - let max_depth = [ - Tableau::Tableau1, - Tableau::Tableau2, - Tableau::Tableau3, - Tableau::Tableau4, - Tableau::Tableau5, - Tableau::Tableau6, - Tableau::Tableau7, - ] - .into_iter() - .map(|tableau| { - game.0 - .pile(KlondikePile::Tableau(tableau)) - .into_iter() - .filter(|(_, face_up)| *face_up) - .count() - }) - .max() - .unwrap_or(0); - - let card_h = layout.0.card_size.y; - let avail = layout.0.available_tableau_height; - - // With ≤ 1 face-up card per column (fresh deal, or completely face-down - // piles) the face-up fan fraction has no visible effect. Leave both fracs - // at the adaptive values set by compute_layout rather than snapping them - // to the desktop minimum. - if max_depth <= 1 || card_h <= 0.0 { - return; - } - - let ideal = avail / ((max_depth - 1) as f32 * card_h); - let max_frac = if card_h > 0.0 { - avail / (12.0 * card_h) - } else { - TABLEAU_FAN_FRAC - }; - let new_frac = ideal.clamp(TABLEAU_FAN_FRAC, max_frac.max(TABLEAU_FAN_FRAC)); - let new_facedown_frac = new_frac * (TABLEAU_FACEDOWN_FAN_FRAC / TABLEAU_FAN_FRAC); - - if (layout.0.tableau_fan_frac - new_frac).abs() > 1e-4 { - layout.0.tableau_fan_frac = new_frac; - } - if (layout.0.tableau_facedown_fan_frac - new_facedown_frac).abs() > 1e-4 { - layout.0.tableau_facedown_fan_frac = new_facedown_frac; - } + crate::layout::apply_dynamic_tableau_fan(&game.0, &mut layout.0); } +/// PostStartup sibling of [`update_tableau_fan_frac`]. The initial deal is +/// inserted directly as `GameStateResource` at startup without a +/// `StateChangedEvent`, so the event-driven system never fires for it. This +/// runs once, before [`sync_cards_startup`] renders, so the very first board +/// (cold start) already fills the viewport — otherwise a fresh deal on a tall / +/// near-square screen (e.g. an unfolded foldable) renders with the unspread fan +/// and a large empty band below the tableau until the first move. +fn fill_tableau_fan_on_startup( + game: Option>, + mut layout: Option>, +) { + let Some(game) = game else { + return; + }; + let Some(layout) = layout.as_mut() else { + return; + }; + crate::layout::apply_dynamic_tableau_fan(&game.0, &mut layout.0); +} + + #[cfg(test)] mod tests { use super::*; use crate::game_plugin::GamePlugin; + use crate::layout::TABLEAU_FAN_FRAC; use crate::table_plugin::TablePlugin; use solitaire_core::Deck; @@ -2975,6 +2963,59 @@ mod tests { } } + #[test] + fn cold_start_deal_fills_tableau_fan() { + // The initial deal is inserted at startup without a StateChangedEvent, so + // the event-driven fan update never fires for it. The PostStartup fill + // must spread the fan from the deepest column's *total* depth (face-down + // included) so a fresh deal already fills the viewport — otherwise a + // near-square / unfolded-foldable screen renders with a large empty band + // below the tableau. Mirrors apply_dynamic_tableau_fan's formula against + // the actual dealt state so it fails if the startup fill is dropped or + // reverts to face-up-only depth. + let app = app(); + + let game = app.world().resource::(); + let facedown_ratio = TABLEAU_FACEDOWN_FAN_FRAC / TABLEAU_FAN_FRAC; + let max_demand = [ + Tableau::Tableau1, + Tableau::Tableau2, + Tableau::Tableau3, + Tableau::Tableau4, + Tableau::Tableau5, + Tableau::Tableau6, + Tableau::Tableau7, + ] + .into_iter() + .map(|t| { + let pile = game.0.pile(KlondikePile::Tableau(t)); + let steps = pile.len().saturating_sub(1); + pile.iter() + .take(steps) + .map(|(_, up)| if *up { 1.0 } else { facedown_ratio }) + .sum::() + }) + .fold(0.0_f32, f32::max); + assert!( + max_demand > 1.0, + "a fresh deal's deepest column should contribute several fan steps, got {max_demand}" + ); + + let layout = app.world().resource::(); + let card_h = layout.0.card_size.y; + let avail = layout.0.available_tableau_height; + let expected = (avail / (max_demand * card_h)) + .clamp(TABLEAU_FAN_FRAC, crate::layout::MAX_DYNAMIC_FAN_FRAC); + + assert!( + (layout.0.tableau_fan_frac - expected).abs() < 1e-3, + "cold-start fan {} should equal the demand-filled value {} \ + (card_h={card_h}, avail={avail}, demand={max_demand})", + layout.0.tableau_fan_frac, + expected, + ); + } + #[test] fn flip_half_secs_is_positive() { const { @@ -3240,6 +3281,56 @@ mod tests { }); } + #[test] + fn resize_keeps_tableau_fan_filled() { + // The Android safe-area-inset update (frames 1-3) and fold/unfold both + // fire a WindowResized that recomputes the layout, resetting the fan to + // compute_layout's sparse worst-case value. on_window_resized must + // re-apply the dynamic fill so the tableau keeps filling the viewport + // after a resize — not only at deal time. Without the fill in the resize + // path the fan would snap back to the worst-case value and the lower + // screen would empty out on the first resize. + let mut app = app(); + + // A tall near-square window (like an unfolded foldable) where the dynamic + // fill clearly exceeds the worst-case fan. + fire_window_resize(&mut app, 1400.0, 1500.0); + advance_past_resize_throttle(&mut app); + + let game = app.world().resource::(); + let facedown_ratio = TABLEAU_FACEDOWN_FAN_FRAC / TABLEAU_FAN_FRAC; + let max_demand = [ + Tableau::Tableau1, + Tableau::Tableau2, + Tableau::Tableau3, + Tableau::Tableau4, + Tableau::Tableau5, + Tableau::Tableau6, + Tableau::Tableau7, + ] + .into_iter() + .map(|t| { + let pile = game.0.pile(KlondikePile::Tableau(t)); + let steps = pile.len().saturating_sub(1); + pile.iter() + .take(steps) + .map(|(_, up)| if *up { 1.0 } else { facedown_ratio }) + .sum::() + }) + .fold(0.0_f32, f32::max); + + let layout = app.world().resource::(); + let expected = (layout.0.available_tableau_height / (max_demand * layout.0.card_size.y)) + .clamp(TABLEAU_FAN_FRAC, crate::layout::MAX_DYNAMIC_FAN_FRAC); + assert!( + (layout.0.tableau_fan_frac - expected).abs() < 1e-3, + "after resize the fan {} should be re-filled to {} (the resize path must \ + re-apply apply_dynamic_tableau_fan)", + layout.0.tableau_fan_frac, + expected, + ); + } + #[test] fn resize_does_not_despawn_card_labels() { // Spawn a fresh app, capture the current set of CardLabel entity IDs, diff --git a/solitaire_engine/src/layout.rs b/solitaire_engine/src/layout.rs index 4863e29..d61d384 100644 --- a/solitaire_engine/src/layout.rs +++ b/solitaire_engine/src/layout.rs @@ -7,6 +7,7 @@ use std::collections::HashMap; use bevy::math::Vec2; use bevy::prelude::{Resource, SystemSet}; +use solitaire_core::game_state::GameState; use solitaire_core::{Foundation, KlondikePile, Tableau}; /// Schedule labels for layout-related systems so cross-plugin ordering is @@ -91,6 +92,15 @@ const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.14; /// this column inside the visible window. const MAX_TABLEAU_CARDS: f32 = 13.0; +/// Upper bound for the dynamic tableau fan step (fraction of card height) chosen +/// by [`apply_dynamic_tableau_fan`]. The fan is spread to fill the available +/// height, but a near-empty column has tiny demand, so without a cap its few +/// cards would fling far apart on a tall viewport. At 0.6 the face-up cards keep +/// clear overlap (a readable stack) while still filling most of a near-square / +/// unfolded-foldable screen. Tunable purely for feel — no effect on correctness +/// or hit-testing. +pub(crate) const MAX_DYNAMIC_FAN_FRAC: f32 = 0.6; + /// Vertical pixel band reserved at the top of the play area for the HUD /// (action buttons, Score / Moves / Timer readouts). The card grid starts /// below this band so the HUD doesn't bleed into the play surface. @@ -309,6 +319,76 @@ pub fn compute_layout( } } +/// Spread the tableau fan so the deepest column fills the available vertical +/// height for the *current* deal, mutating `layout.tableau_fan_frac` and its +/// face-down companion in place. +/// +/// `compute_layout` is pure geometry and sizes the fan for a worst-case 13-card +/// column, so early in a game (shallow columns) a tall or near-square viewport +/// — e.g. an unfolded foldable — is left with a large empty band below the +/// tableau. This refines the fan once the actual deal is known. +/// +/// Depth is measured across *all* cards in a column, each face-down card +/// weighted by the fixed face-down/face-up step ratio. Counting the face-down +/// portion (not just the face-up tail) is what fills the lower screen on a fresh +/// deal, where the deepest column is several face-down cards under one face-up +/// one. Deeper columns drive the fraction down so everything still fits; +/// [`TABLEAU_FAN_FRAC`] floors it and [`MAX_DYNAMIC_FAN_FRAC`] caps it. +/// +/// Called from card-sync at startup and on every `StateChangedEvent`, and from +/// the resize pipeline after `compute_layout`, so the cold-start deal, ongoing +/// play, and fold/unfold all stay filled. `card_position` / `card_positions` +/// read the same fractions, so rendering and hit-testing remain in sync. +pub(crate) fn apply_dynamic_tableau_fan(game: &GameState, layout: &mut Layout) { + let card_h = layout.card_size.y; + let avail = layout.available_tableau_height; + if card_h <= 0.0 { + return; + } + + let facedown_ratio = TABLEAU_FACEDOWN_FAN_FRAC / TABLEAU_FAN_FRAC; + + // "Step demand" of a column: the vertical offset of its bottom card from its + // top card, in units of the face-up fan step. Every card except the last + // contributes one step, weighted down to `facedown_ratio` while face-down. + let max_demand = [ + Tableau::Tableau1, + Tableau::Tableau2, + Tableau::Tableau3, + Tableau::Tableau4, + Tableau::Tableau5, + Tableau::Tableau6, + Tableau::Tableau7, + ] + .into_iter() + .map(|tableau| { + let pile = game.pile(KlondikePile::Tableau(tableau)); + let steps = pile.len().saturating_sub(1); + pile.iter() + .take(steps) + .map(|(_, face_up)| if *face_up { 1.0 } else { facedown_ratio }) + .sum::() + }) + .fold(0.0_f32, f32::max); + + // No fannable column (every tableau pile has ≤ 1 card) — leave the fractions + // at the values compute_layout set. + if max_demand <= 0.0 { + return; + } + + let ideal = avail / (max_demand * card_h); + let new_frac = ideal.clamp(TABLEAU_FAN_FRAC, MAX_DYNAMIC_FAN_FRAC); + let new_facedown_frac = new_frac * facedown_ratio; + + if (layout.tableau_fan_frac - new_frac).abs() > 1e-4 { + layout.tableau_fan_frac = new_frac; + } + if (layout.tableau_facedown_fan_frac - new_facedown_frac).abs() > 1e-4 { + layout.tableau_facedown_fan_frac = new_facedown_frac; + } +} + /// Bevy resource wrapping the current `Layout`. Recomputed on `WindowResized`. #[derive(Resource, Debug, Clone)] pub struct LayoutResource(pub Layout); diff --git a/solitaire_engine/src/table_plugin.rs b/solitaire_engine/src/table_plugin.rs index 854f4d3..82ca538 100644 --- a/solitaire_engine/src/table_plugin.rs +++ b/solitaire_engine/src/table_plugin.rs @@ -11,7 +11,9 @@ use solitaire_core::Suit; use crate::events::{HintVisualEvent, StateChangedEvent}; use crate::hud_plugin::HudVisibility; -use crate::layout::{Layout, LayoutResource, LayoutSystem, TABLE_COLOUR, compute_layout}; +use crate::layout::{ + Layout, LayoutResource, LayoutSystem, TABLE_COLOUR, apply_dynamic_tableau_fan, compute_layout, +}; use crate::resources::GameStateResource; use crate::safe_area::SafeAreaInsets; use crate::settings_plugin::{SettingsChangedEvent, SettingsResource}; @@ -348,12 +350,13 @@ fn spawn_pile_markers(commands: &mut Commands, layout: &Layout) { } } -#[allow(clippy::type_complexity)] +#[allow(clippy::type_complexity, clippy::too_many_arguments)] fn on_window_resized( mut events: MessageReader, safe_area: Option>, windows: Query<&Window>, hud_vis: Option>, + game: Option>, mut layout_res: Option>, mut backgrounds: Query< (&mut Sprite, &mut Transform), @@ -370,7 +373,16 @@ fn on_window_resized( let safe_area_top = insets.top / scale; let safe_area_bottom = insets.bottom / scale; let hud_visible = hud_vis.as_deref().copied().unwrap_or_default() == HudVisibility::Visible; - let new_layout = compute_layout(window_size, safe_area_top, safe_area_bottom, hud_visible); + let mut new_layout = compute_layout(window_size, safe_area_top, safe_area_bottom, hud_visible); + + // compute_layout sizes the fan for a worst-case column; refine it to the + // current deal so a resize (incl. the Android safe-area-inset resize that + // fires in the first few frames, and fold/unfold on foldables) keeps the + // tableau filling the viewport instead of snapping back to the sparse + // worst-case fan. + if let Some(game) = game.as_ref() { + apply_dynamic_tableau_fan(&game.0, &mut new_layout); + } if let Some(layout_res) = layout_res.as_deref_mut() { layout_res.0 = new_layout.clone();