fix(engine): correct Android waste fan overlap and resume layout desync
Android Release / build-apk (push) Successful in 4m41s
Android Release / build-apk (push) Successful in 4m41s
Bug 1 (card_plugin): waste Draw-Three fan step was a fixed 0.28×card_width, chosen for the desktop gap ratio (H_GAP_DIVISOR=4). On Android (H_GAP_DIVISOR=32) the column spacing is only 1.031×card_width, so the same fraction pushed the top fanned card's centre past the waste column's right edge. Fix: derive fan_step from column spacing × 0.224 — preserves 0.28×cw on desktop while reducing to ≈0.231×cw on Android, keeping fanned cards within their column footprint. Adds regression test on 900×2000 portrait window. Bug 2 (safe_area): refresh_insets stored its retry counter as Local<u32>, making it impossible to re-arm after a background/foreground cycle. On resume the counter was already saturated so JNI was never re-queried; layouts computed with stale (zero) insets pushed the top card row up under the HUD. Fix: convert tries to SafeAreaPollTries Resource; add android::rearm_on_resumed which resets both counter and SafeAreaInsets on AppLifecycle::WillResume so the poller re-fires; add on_app_resumed (all platforms) which emits a synthetic WindowResized on WillResume to immediately trigger layout recomputation. Adds pure-function regression test in layout.rs pinning the suspend→resume invariant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -742,6 +742,19 @@ fn card_positions<'a>(game: &'a GameState, layout: &Layout) -> Vec<(&'a Card, Ve
|
|||||||
PileType::Tableau(6),
|
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 {
|
for pile_type in piles {
|
||||||
let Some(base) = layout.pile_positions.get(&pile_type) else {
|
let Some(base) = layout.pile_positions.get(&pile_type) else {
|
||||||
continue;
|
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.
|
// normally — no card is hidden, so the shift is 0.
|
||||||
let visible = 3_usize;
|
let visible = 3_usize;
|
||||||
let hidden = rendered_len.saturating_sub(visible);
|
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 {
|
} else {
|
||||||
0.0
|
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<u32> = 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]
|
#[test]
|
||||||
fn waste_pile_draw_one_cards_have_distinct_z() {
|
fn waste_pile_draw_one_cards_have_distinct_z() {
|
||||||
use solitaire_core::game_state::DrawMode;
|
use solitaire_core::game_state::DrawMode;
|
||||||
|
|||||||
@@ -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.
|
/// safe_area_bottom must not affect horizontal positions.
|
||||||
#[test]
|
#[test]
|
||||||
fn safe_area_bottom_does_not_affect_horizontal_layout() {
|
fn safe_area_bottom_does_not_affect_horizontal_layout() {
|
||||||
|
|||||||
@@ -18,6 +18,7 @@
|
|||||||
//! changes flow through automatically.
|
//! changes flow through automatically.
|
||||||
|
|
||||||
use bevy::prelude::*;
|
use bevy::prelude::*;
|
||||||
|
use bevy::window::{AppLifecycle, WindowResized};
|
||||||
|
|
||||||
use crate::ui_modal::ModalScrim;
|
use crate::ui_modal::ModalScrim;
|
||||||
|
|
||||||
@@ -65,14 +66,25 @@ pub struct SafeAreaInsetsPlugin;
|
|||||||
|
|
||||||
impl Plugin for SafeAreaInsetsPlugin {
|
impl Plugin for SafeAreaInsetsPlugin {
|
||||||
fn build(&self, app: &mut App) {
|
fn build(&self, app: &mut App) {
|
||||||
app.init_resource::<SafeAreaInsets>()
|
// Both message types may already be registered by GamePlugin / TablePlugin;
|
||||||
|
// add_message is idempotent.
|
||||||
|
app.add_message::<AppLifecycle>()
|
||||||
|
.add_message::<WindowResized>()
|
||||||
|
.init_resource::<SafeAreaInsets>()
|
||||||
.add_systems(
|
.add_systems(
|
||||||
Update,
|
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")]
|
#[cfg(target_os = "android")]
|
||||||
app.add_systems(Update, android::refresh_insets);
|
app.init_resource::<android::SafeAreaPollTries>()
|
||||||
|
.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<AppLifecycle>,
|
||||||
|
windows: Query<(Entity, &Window)>,
|
||||||
|
mut resize_events: MessageWriter<WindowResized>,
|
||||||
|
) {
|
||||||
|
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")]
|
#[cfg(target_os = "android")]
|
||||||
mod android {
|
mod android {
|
||||||
use super::SafeAreaInsets;
|
use super::{AppLifecycle, SafeAreaInsets};
|
||||||
use bevy::prelude::*;
|
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
|
/// Polls Android for safe-area insets until we get a non-zero
|
||||||
/// reading, then stops. `getRootWindowInsets()` returns `null` (or
|
/// reading, then stops. `getRootWindowInsets()` returns `null` (or
|
||||||
/// all-zero `Insets`) until the decor view has been laid out, which
|
/// all-zero `Insets`) until the decor view has been laid out, which
|
||||||
/// is typically frame 1–3 of a fresh launch.
|
/// is typically frame 1–3 of a fresh launch.
|
||||||
pub(super) fn refresh_insets(
|
pub(super) fn refresh_insets(
|
||||||
mut insets: ResMut<SafeAreaInsets>,
|
mut insets: ResMut<SafeAreaInsets>,
|
||||||
mut tries: Local<u32>,
|
mut poll: ResMut<SafeAreaPollTries>,
|
||||||
) {
|
) {
|
||||||
// Cap retries so we don't burn CPU forever on edge-to-edge
|
// Cap retries so we don't burn CPU forever on edge-to-edge
|
||||||
// devices that genuinely report zero insets.
|
// devices that genuinely report zero insets.
|
||||||
const MAX_TRIES: u32 = 120; // ~2 seconds @ 60 fps
|
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;
|
return;
|
||||||
}
|
}
|
||||||
*tries += 1;
|
poll.0 += 1;
|
||||||
|
|
||||||
match query_insets() {
|
match query_insets() {
|
||||||
Ok(v) if v.is_populated() => {
|
Ok(v) if v.is_populated() => {
|
||||||
info!(
|
info!(
|
||||||
"safe_area: insets resolved top={} bottom={} left={} right={} (after {} frames)",
|
"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;
|
*insets = v;
|
||||||
}
|
}
|
||||||
@@ -177,13 +229,35 @@ mod android {
|
|||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
// Don't spam — log once and let polling continue silently.
|
// 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}");
|
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<AppLifecycle>,
|
||||||
|
mut poll: ResMut<SafeAreaPollTries>,
|
||||||
|
mut insets: ResMut<SafeAreaInsets>,
|
||||||
|
) {
|
||||||
|
for event in lifecycle.read() {
|
||||||
|
if matches!(event, AppLifecycle::WillResume) {
|
||||||
|
poll.0 = 0;
|
||||||
|
*insets = SafeAreaInsets::default();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn query_insets() -> Result<SafeAreaInsets, String> {
|
fn query_insets() -> Result<SafeAreaInsets, String> {
|
||||||
use bevy::android::ANDROID_APP;
|
use bevy::android::ANDROID_APP;
|
||||||
use jni::{objects::JObject, JavaVM};
|
use jni::{objects::JObject, JavaVM};
|
||||||
|
|||||||
Reference in New Issue
Block a user