feat(engine): solver-vetted seed selection on AsyncComputeTaskPool
"Winnable deals only" used to call `choose_winnable_seed` inline on the main thread inside `handle_new_game`. Each rejected attempt costs ~120 ms (`SolverConfig::default()` budget); the loop caps at `SOLVER_DEAL_RETRY_CAP` = 50, so a pathological run could stall the UI for ~6 s on a New Game click. Quat flagged this as the highest- impact UX regression left in the engine. Reorganised so the solver runs on `AsyncComputeTaskPool`: - New `PendingNewGameSeed` resource holds an `Option<PendingSeedTask>` carrying the in-flight `Task<u64>` plus the request's `mode` and `confirmed` flags so the polling system can replay them on a synthetic `NewGameRequestEvent` once the task resolves. - `handle_new_game` now writes to that resource (and `continue`s) for the winnable-only / Classic / random-seed branch, instead of calling `choose_winnable_seed` synchronously. - `poll_pending_new_game_seed` runs `.before(GameMutation)` so the synthetic event lands in the same frame's `handle_new_game` — the player sees no extra-frame visual lag once the solver completes. - Cancel-on-replace: when a fresh `NewGameRequestEvent` arrives while a previous task is in flight, `pending_seed.inner = None` drops the old task (Bevy's `Task` Drop cancels cooperatively at the next await point) before processing the new request. Two tests: - `winnable_seed_search_runs_async_and_completes_eventually` — spawns the task, drives `app.update()` in a wall-clock-bounded loop with `std::thread::yield_now()` so the shared `AsyncComputeTaskPool` gets a chance to schedule between polls. - `winnable_seed_search_drops_in_flight_task_on_new_request` — fires a winnable-only request, then before the task can complete fires an explicit-seed request that bypasses the solver entirely. Asserts the explicit seed wins, verifying the cancel-on-replace contract. Existing solver tests pass unchanged: explicit-seed paths skip the new branch and run synchronously like before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ use std::path::PathBuf;
|
|||||||
use std::time::{SystemTime, UNIX_EPOCH};
|
use std::time::{SystemTime, UNIX_EPOCH};
|
||||||
|
|
||||||
use bevy::prelude::*;
|
use bevy::prelude::*;
|
||||||
|
use bevy::tasks::{futures_lite::future, AsyncComputeTaskPool, Task};
|
||||||
use chrono::Utc;
|
use chrono::Utc;
|
||||||
use solitaire_core::game_state::{DrawMode, GameMode, GameState};
|
use solitaire_core::game_state::{DrawMode, GameMode, GameState};
|
||||||
use solitaire_core::pile::PileType;
|
use solitaire_core::pile::PileType;
|
||||||
@@ -137,6 +138,7 @@ impl Plugin for GamePlugin {
|
|||||||
.insert_resource(GameStatePath(path))
|
.insert_resource(GameStatePath(path))
|
||||||
.insert_resource(ReplayPath(history_path))
|
.insert_resource(ReplayPath(history_path))
|
||||||
.init_resource::<RecordingReplay>()
|
.init_resource::<RecordingReplay>()
|
||||||
|
.init_resource::<PendingNewGameSeed>()
|
||||||
.init_resource::<DragState>()
|
.init_resource::<DragState>()
|
||||||
.init_resource::<SyncStatusResource>()
|
.init_resource::<SyncStatusResource>()
|
||||||
.add_message::<MoveRequestEvent>()
|
.add_message::<MoveRequestEvent>()
|
||||||
@@ -150,6 +152,10 @@ impl Plugin for GamePlugin {
|
|||||||
.add_message::<crate::events::AchievementUnlockedEvent>()
|
.add_message::<crate::events::AchievementUnlockedEvent>()
|
||||||
.add_message::<FoundationCompletedEvent>()
|
.add_message::<FoundationCompletedEvent>()
|
||||||
.add_message::<InfoToastEvent>()
|
.add_message::<InfoToastEvent>()
|
||||||
|
.add_systems(
|
||||||
|
Update,
|
||||||
|
poll_pending_new_game_seed.before(GameMutation),
|
||||||
|
)
|
||||||
.add_systems(
|
.add_systems(
|
||||||
Update,
|
Update,
|
||||||
(
|
(
|
||||||
@@ -237,6 +243,60 @@ fn seed_from_system_time() -> u64 {
|
|||||||
/// seed so the player still gets a deal — better a possibly-unwinnable
|
/// seed so the player still gets a deal — better a possibly-unwinnable
|
||||||
/// hand than an infinite loop.
|
/// hand than an infinite loop.
|
||||||
///
|
///
|
||||||
|
/// In-flight async work for "Winnable deals only" seed selection.
|
||||||
|
///
|
||||||
|
/// `handle_new_game` writes here when it needs the solver to vet a deal;
|
||||||
|
/// `poll_pending_new_game_seed` reads from here, polls the task, and
|
||||||
|
/// re-emits a `NewGameRequestEvent` with the chosen seed once the task
|
||||||
|
/// completes. The desktop client's UI never blocks on the worst-case
|
||||||
|
/// 50 × ~120 ms solver runs that can pile up on pathological deals.
|
||||||
|
///
|
||||||
|
/// At most one task is ever in flight: a fresh new-game request while
|
||||||
|
/// a previous task is still running drops the previous task (Bevy's
|
||||||
|
/// `Task` `Drop` cancels it cooperatively at the next await point) and
|
||||||
|
/// queues the new one.
|
||||||
|
#[derive(Resource, Default)]
|
||||||
|
pub struct PendingNewGameSeed {
|
||||||
|
/// `Some` while a solver-vetted seed is being computed.
|
||||||
|
inner: Option<PendingSeedTask>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// One in-flight winnable-seed search plus the request fields that
|
||||||
|
/// would have flowed through `handle_new_game` synchronously. The
|
||||||
|
/// poll system replays them on a synthetic `NewGameRequestEvent` once
|
||||||
|
/// the task completes — `seed: Some(...)` skips the solver branch on
|
||||||
|
/// the second pass so we don't loop.
|
||||||
|
struct PendingSeedTask {
|
||||||
|
handle: Task<u64>,
|
||||||
|
mode: Option<GameMode>,
|
||||||
|
confirmed: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Update system: poll the in-flight winnable-seed search. When the
|
||||||
|
/// task resolves, emit a synthetic `NewGameRequestEvent` carrying the
|
||||||
|
/// chosen seed. Ordered `.before(GameMutation)` so `handle_new_game`
|
||||||
|
/// picks up the synthetic event on the same frame, completing the
|
||||||
|
/// new-game flow without a one-frame visual lag.
|
||||||
|
fn poll_pending_new_game_seed(
|
||||||
|
mut pending: ResMut<PendingNewGameSeed>,
|
||||||
|
mut new_game_writer: MessageWriter<NewGameRequestEvent>,
|
||||||
|
) {
|
||||||
|
let Some(p) = pending.inner.as_mut() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let Some(seed) = future::block_on(future::poll_once(&mut p.handle)) else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let mode = p.mode;
|
||||||
|
let confirmed = p.confirmed;
|
||||||
|
pending.inner = None;
|
||||||
|
new_game_writer.write(NewGameRequestEvent {
|
||||||
|
seed: Some(seed),
|
||||||
|
mode,
|
||||||
|
confirmed,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
/// Pure helper extracted for testability — `new_game_with_solver_*`
|
/// Pure helper extracted for testability — `new_game_with_solver_*`
|
||||||
/// engine tests in the same file exercise this path.
|
/// engine tests in the same file exercise this path.
|
||||||
pub(crate) fn choose_winnable_seed(initial_seed: u64, draw_mode: &DrawMode) -> u64 {
|
pub(crate) fn choose_winnable_seed(initial_seed: u64, draw_mode: &DrawMode) -> u64 {
|
||||||
@@ -262,6 +322,7 @@ fn handle_new_game(
|
|||||||
mut game: ResMut<GameStateResource>,
|
mut game: ResMut<GameStateResource>,
|
||||||
mut changed: MessageWriter<StateChangedEvent>,
|
mut changed: MessageWriter<StateChangedEvent>,
|
||||||
mut recording: ResMut<RecordingReplay>,
|
mut recording: ResMut<RecordingReplay>,
|
||||||
|
mut pending_seed: ResMut<PendingNewGameSeed>,
|
||||||
settings: Option<Res<crate::settings_plugin::SettingsResource>>,
|
settings: Option<Res<crate::settings_plugin::SettingsResource>>,
|
||||||
path: Option<Res<GameStatePath>>,
|
path: Option<Res<GameStatePath>>,
|
||||||
font_res: Option<Res<FontResource>>,
|
font_res: Option<Res<FontResource>>,
|
||||||
@@ -296,6 +357,13 @@ fn handle_new_game(
|
|||||||
commands.entity(entity).despawn();
|
commands.entity(entity).despawn();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Drop any in-flight winnable-seed search now that we've
|
||||||
|
// committed to acting on a new request. Its result was for
|
||||||
|
// the previous user intent — the new request supersedes it
|
||||||
|
// regardless of which branch we take below (synchronous
|
||||||
|
// explicit-seed deal vs. another async solver search).
|
||||||
|
pending_seed.inner = None;
|
||||||
|
|
||||||
let initial_seed = ev.seed.unwrap_or_else(seed_from_system_time);
|
let initial_seed = ev.seed.unwrap_or_else(seed_from_system_time);
|
||||||
// Prefer the draw mode from Settings when starting a fresh game.
|
// Prefer the draw mode from Settings when starting a fresh game.
|
||||||
// Fall back to the current game's draw mode in headless/test contexts
|
// Fall back to the current game's draw mode in headless/test contexts
|
||||||
@@ -323,11 +391,22 @@ fn handle_new_game(
|
|||||||
let winnable_only = settings
|
let winnable_only = settings
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.is_some_and(|s| s.0.winnable_deals_only);
|
.is_some_and(|s| s.0.winnable_deals_only);
|
||||||
let chosen_seed = if winnable_only && mode == GameMode::Classic && ev.seed.is_none() {
|
if winnable_only && mode == GameMode::Classic && ev.seed.is_none() {
|
||||||
choose_winnable_seed(initial_seed, &draw_mode)
|
let dm = draw_mode.clone();
|
||||||
} else {
|
let task = AsyncComputeTaskPool::get()
|
||||||
initial_seed
|
.spawn(async move { choose_winnable_seed(initial_seed, &dm) });
|
||||||
};
|
pending_seed.inner = Some(PendingSeedTask {
|
||||||
|
handle: task,
|
||||||
|
mode: ev.mode,
|
||||||
|
confirmed: ev.confirmed,
|
||||||
|
});
|
||||||
|
// Skip the rest of the new-game flow; the polling system
|
||||||
|
// will re-emit a synthetic event with a chosen seed once
|
||||||
|
// the task resolves.
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
let chosen_seed = initial_seed;
|
||||||
|
|
||||||
game.0 = GameState::new_with_mode(chosen_seed, draw_mode, mode);
|
game.0 = GameState::new_with_mode(chosen_seed, draw_mode, mode);
|
||||||
// Reset the in-flight replay buffer — a fresh deal starts with
|
// Reset the in-flight replay buffer — a fresh deal starts with
|
||||||
@@ -2320,4 +2399,111 @@ mod tests {
|
|||||||
0
|
0
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Async-solver flow: a winnable-only request with no explicit
|
||||||
|
/// seed must populate `PendingNewGameSeed` on the same frame the
|
||||||
|
/// request fires (no main-thread stall waiting on the solver),
|
||||||
|
/// and subsequent updates must clear the pending state and
|
||||||
|
/// produce a new GameState.
|
||||||
|
///
|
||||||
|
/// Drives multiple `app.update()` calls because the polling
|
||||||
|
/// system needs at least one tick after spawn to observe the
|
||||||
|
/// task as ready and re-emit the synthetic event.
|
||||||
|
#[test]
|
||||||
|
fn winnable_seed_search_runs_async_and_completes_eventually() {
|
||||||
|
let mut app = test_app(394);
|
||||||
|
insert_settings(&mut app, true);
|
||||||
|
|
||||||
|
app.world_mut().write_message(NewGameRequestEvent {
|
||||||
|
seed: None,
|
||||||
|
mode: None,
|
||||||
|
confirmed: false,
|
||||||
|
});
|
||||||
|
// First update: handle_new_game spawns the solver task and
|
||||||
|
// returns. The GameStateResource is unchanged on this tick —
|
||||||
|
// the player's previous game is still on screen, so the UI
|
||||||
|
// doesn't visually stall.
|
||||||
|
app.update();
|
||||||
|
assert!(
|
||||||
|
app.world().resource::<PendingNewGameSeed>().inner.is_some(),
|
||||||
|
"first frame should have an in-flight solver task",
|
||||||
|
);
|
||||||
|
|
||||||
|
// Pump frames until the polling system observes the task as
|
||||||
|
// ready and re-emits the synthetic event. AsyncComputeTaskPool
|
||||||
|
// is a shared pool across the whole `cargo test` run — when
|
||||||
|
// dozens of tests execute in parallel the pool can take a
|
||||||
|
// while to actually schedule our future. The yield_now() lets
|
||||||
|
// the pool's worker threads make progress between our polls
|
||||||
|
// without burning wall-clock time.
|
||||||
|
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(15);
|
||||||
|
while app.world().resource::<PendingNewGameSeed>().inner.is_some() {
|
||||||
|
app.update();
|
||||||
|
std::thread::yield_now();
|
||||||
|
if std::time::Instant::now() >= deadline {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
app.world().resource::<PendingNewGameSeed>().inner.is_none(),
|
||||||
|
"solver task should have completed within 15 s wall-clock",
|
||||||
|
);
|
||||||
|
// New game completed: a fresh deal carries 0 moves.
|
||||||
|
assert_eq!(
|
||||||
|
app.world().resource::<GameStateResource>().0.move_count,
|
||||||
|
0,
|
||||||
|
"completed new game must be in fresh-deal state",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Cancel-on-replace: a winnable-only request that arrives while
|
||||||
|
/// a previous solver task is in flight must drop the previous
|
||||||
|
/// task and queue the new one. The most recently-fired request
|
||||||
|
/// is the one whose seed wins, regardless of which task started
|
||||||
|
/// first.
|
||||||
|
#[test]
|
||||||
|
fn winnable_seed_search_drops_in_flight_task_on_new_request() {
|
||||||
|
let mut app = test_app(394);
|
||||||
|
insert_settings(&mut app, true);
|
||||||
|
|
||||||
|
// Fire the first request; first update spawns the task.
|
||||||
|
app.world_mut().write_message(NewGameRequestEvent {
|
||||||
|
seed: None,
|
||||||
|
mode: None,
|
||||||
|
confirmed: false,
|
||||||
|
});
|
||||||
|
app.update();
|
||||||
|
assert!(
|
||||||
|
app.world().resource::<PendingNewGameSeed>().inner.is_some(),
|
||||||
|
"first request should be in flight",
|
||||||
|
);
|
||||||
|
|
||||||
|
// Fire a SECOND request with an explicit seed before the
|
||||||
|
// first task can complete. handle_new_game's `pending.inner =
|
||||||
|
// None` line must drop the in-flight task; the explicit-seed
|
||||||
|
// branch then bypasses the solver entirely. After this tick
|
||||||
|
// the GameStateResource carries seed 12345, not whatever the
|
||||||
|
// solver would have picked for the first request.
|
||||||
|
app.world_mut().write_message(NewGameRequestEvent {
|
||||||
|
seed: Some(12345),
|
||||||
|
mode: None,
|
||||||
|
confirmed: true,
|
||||||
|
});
|
||||||
|
app.update();
|
||||||
|
|
||||||
|
// Drive a few more ticks to drain any stragglers.
|
||||||
|
for _ in 0..5 {
|
||||||
|
app.update();
|
||||||
|
}
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
app.world().resource::<PendingNewGameSeed>().inner.is_none(),
|
||||||
|
"explicit-seed request must have cancelled the in-flight task",
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
app.world().resource::<GameStateResource>().0.seed,
|
||||||
|
12345,
|
||||||
|
"explicit-seed request takes precedence over the dropped solver task",
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user