funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 48 Wiki Activity

refactor(core): card_game redundancy cleanup + derive scoring from upstream stats #88

Merged
funman300 merged 16 commits from refactor/strip-card_game-redundancies into master 2026-06-22 18:44:38 +00:00
Conversation 0 Commits 16 Files Changed 88
+1541 -2073
funman300 commented 2026-06-11 04:12:13 +00:00
Owner
Copy Link
Copy Source

✅ Merged to master on 2026-06-22 as merge commit dde65a7.

Completes the card_game/klondike migration arc — issues #82–#91 are all closed,
including #91 (workspace forbid(unsafe) with all Android JNI FFI quarantined behind
the safe solitaire_data::android_jni bridge in solitaire_app). The feature branch
has been deleted.

Summary

Finishes the card_game/klondike migration and the post-migration cleanup. Four substantive strands on this branch:

  1. Redundancy cleanup — small remaining duplications removed after the migration.
  2. Scoring migration (#84, #86, #87) — GameState's hand-rolled WXP scoring state replaced by values derived from the upstream session stats (unblocked by card_game 0.4.1 / klondike 0.4.0).
  3. CardEntityIndex perf — engine drag card→entity lookups moved from O(n) query scans to an O(1) index.
  4. Solver wrapper deletion — solitaire_data::solver removed; solving goes through card_game::Session::solve() via a core domain method.

Changes

Redundancy cleanup

  • core: delete dead GameState::compute_time_bonus
  • data: drop dead load_latest_replay_from/save_latest_replay_to re-exports
  • data+engine: share win-XP scoring via one XpBreakdown
  • engine: use canonical klondike_adapter::foundation_from_slot

Scoring migration — 372b642 (closes #84, #86, #87)

  • Remove GameState fields score/undo_count/recycle_count (#87), the score_history/is_recycle_history undo journal (#86), and KlondikeAdapter::apply_undo_score + score_for_* + pre_instruction_score_delta (#84)
  • Add derived score()/undo_count()/recycle_count() reading session.stats(); −15 undo penalty now SessionConfig::undo_penalty
  • Save schema v4 → v5 (counters no longer persisted; rebuilt by replay; v3/v4 saves still load)
  • Intentional behaviour changes: upstream linear scoring (no escalating recycle penalty, single floor at 0); recycle_count cumulative; undo_count resets across save/load

CardEntityIndex perf — 424c8b2

  • Route the remaining 7 runtime drag card→entity lookups (follow_drag, end_drag, touch_*, update_drag_shadow) through CardEntityIndex (O(1) HashMap) instead of Query::iter().find() — each previously ran per dragged card per frame
  • InputPlugin defensively init_resource::<CardEntityIndex>() (idempotent) so it is self-sufficient in tests

Solver wrapper deletion — e841a7a

  • Remove the standalone solitaire_data::solver module; its shaping (build a solve-budgeted Session, run card_game::Session::solve(), extract the first useful move) moves onto the domain type as GameState::solve_first_move() / solve_fresh_deal() in solitaire_core, with the budget consts + SolveOutcome re-exported from core
  • Solving is deterministic IO-free game logic, so core (which owns GameState) is its correct home; solitaire_data is the persistence layer
  • All consumers (engine hint/new-game/play-by-seed, assetgen seed binaries) call the core API directly

Verification

cargo test --workspace (all crates green) and cargo clippy --workspace --all-targets -- -D warnings clean.

🤖 Generated with Claude Code

> **✅ Merged to `master` on 2026-06-22** as merge commit `dde65a7`. > > Completes the card_game/klondike migration arc — issues **#82–#91** are all closed, > including #91 (workspace `forbid(unsafe)` with all Android JNI FFI quarantined behind > the safe `solitaire_data::android_jni` bridge in `solitaire_app`). The feature branch > has been deleted. --- ## Summary Finishes the `card_game`/`klondike` migration and the post-migration cleanup. Four substantive strands on this branch: 1. **Redundancy cleanup** — small remaining duplications removed after the migration. 2. **Scoring migration (#84, #86, #87)** — `GameState`'s hand-rolled WXP scoring state replaced by values derived from the upstream session stats (unblocked by `card_game 0.4.1` / `klondike 0.4.0`). 3. **`CardEntityIndex` perf** — engine drag card→entity lookups moved from O(n) query scans to an O(1) index. 4. **Solver wrapper deletion** — `solitaire_data::solver` removed; solving goes through `card_game::Session::solve()` via a core domain method. ## Changes ### Redundancy cleanup - **core:** delete dead `GameState::compute_time_bonus` - **data:** drop dead `load_latest_replay_from`/`save_latest_replay_to` re-exports - **data+engine:** share win-XP scoring via one `XpBreakdown` - **engine:** use canonical `klondike_adapter::foundation_from_slot` ### Scoring migration — `372b642` (closes #84, #86, #87) - Remove `GameState` fields `score`/`undo_count`/`recycle_count` (#87), the `score_history`/`is_recycle_history` undo journal (#86), and `KlondikeAdapter::apply_undo_score` + `score_for_*` + `pre_instruction_score_delta` (#84) - Add derived `score()`/`undo_count()`/`recycle_count()` reading `session.stats()`; −15 undo penalty now `SessionConfig::undo_penalty` - Save schema **v4 → v5** (counters no longer persisted; rebuilt by replay; v3/v4 saves still load) - **Intentional behaviour changes:** upstream linear scoring (no escalating recycle penalty, single floor at 0); `recycle_count` cumulative; `undo_count` resets across save/load ### CardEntityIndex perf — `424c8b2` - Route the remaining 7 runtime drag card→entity lookups (`follow_drag`, `end_drag`, `touch_*`, `update_drag_shadow`) through `CardEntityIndex` (O(1) `HashMap`) instead of `Query::iter().find()` — each previously ran per dragged card per frame - `InputPlugin` defensively `init_resource::<CardEntityIndex>()` (idempotent) so it is self-sufficient in tests ### Solver wrapper deletion — `e841a7a` - Remove the standalone `solitaire_data::solver` module; its shaping (build a solve-budgeted `Session`, run `card_game::Session::solve()`, extract the first useful move) moves onto the domain type as `GameState::solve_first_move()` / `solve_fresh_deal()` in `solitaire_core`, with the budget consts + `SolveOutcome` re-exported from core - Solving is deterministic IO-free game logic, so core (which owns `GameState`) is its correct home; `solitaire_data` is the persistence layer - All consumers (engine hint/new-game/play-by-seed, assetgen seed binaries) call the core API directly ## Verification `cargo test --workspace` (all crates green) and `cargo clippy --workspace --all-targets -- -D warnings` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
funman300 added 1 commit 2026-06-11 04:12:14 +00:00
refactor: remove leftover redundancies after card_game migration f0832f3dfa 
Post-migration audit found the card_game/klondike migration essentially
complete; these are the four small redundancies that remained:

- core: delete dead GameState::compute_time_bonus (zero callers; engine
  uses the klondike_adapter free fn directly)
- data: drop dead public re-exports load_latest_replay_from /
  save_latest_replay_to (no callers outside replay.rs); keep
  latest_replay_path (engine legacy migration still uses it)
- data+engine: lift win-XP scoring into a shared XpBreakdown so the
  win-summary modal breakdown and xp_for_win share one source of truth
  instead of duplicating the speed/no-undo constants
- engine: replace feedback_anim_plugin's private foundation_from_slot
  copy with the canonical klondike_adapter::foundation_from_slot

cargo test --workspace + clippy -D warnings green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-11 04:17:06 +00:00
chore: gitignore local agent-tooling artifacts 9e3c6b06b0 
Keep Codex / claude-flow scaffolding (.agents/, .codex/, AGENTS.md) out
of the repo — these are locally generated and not project sources.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-11 17:49:22 +00:00
refactor(core): derive score/undo/recycle from upstream session stats 372b6423d8 
Replace the bespoke WXP scoring engine with the upstream
card_game/klondike session stats, eliminating duplicated state that
could drift from the single source of truth.

score()/undo_count()/recycle_count() now read session.stats(); the -15
undo penalty is configured as SessionConfig::undo_penalty and applied by
the upstream score formula. Save schema bumped v4 -> v5 (the three
counters are no longer persisted -- they are rebuilt by replaying the
forward instruction history on load).

- Remove GameState fields score, undo_count, recycle_count (#87)
- Remove score_history / is_recycle_history undo journal (#86)
- Remove KlondikeAdapter::apply_undo_score and the score_for_* helpers,
  plus pre_instruction_score_delta / will_flip_tableau_source (#84)

These three issues are a single atomic change: each removed field/helper
is consumed by the same draw/apply_instruction/undo/serde/PartialEq
paths, so they cannot compile or pass tests in isolation.

Behaviour changes (intentional): the escalating recycle penalty and
per-step score floor are gone (upstream linear scoring, floored once at
0); recycle_count is now cumulative; undo_count resets across save/load.

Refs #84, #86, #87

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 changed title from refactor: remove leftover redundancies after card_game migration to refactor(core): card_game redundancy cleanup + derive scoring from upstream stats 2026-06-11 17:53:46 +00:00
funman300 added 1 commit 2026-06-11 18:06:32 +00:00
perf(engine): route remaining drag card→entity lookups through CardEntityIndex 424c8b2d50 
Replace O(n) `Query::iter().find()` card scans with O(1) `CardEntityIndex`
lookups in the mouse and touch drag pipelines (`follow_drag`, `end_drag`,
`touch_follow_drag`, `touch_end_drag`) and `update_drag_shadow` — 7 sites
across 5 systems. Each ran per dragged card per frame during a drag.

`InputPlugin` now defensively `init_resource::<CardEntityIndex>()` (idempotent;
`CardPlugin` still owns and rebuilds it) so the plugin is self-sufficient in
tests. The lone remaining card-keyed `.find` is a `#[cfg(test)]` world-query
helper, which is the correct pattern there.

Completes the CardEntityIndex migration started in ef1efdc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 2 commits 2026-06-11 22:11:06 +00:00
refactor: delete solitaire_data::solver wrapper; solve via card_game directly e841a7ab4f 
Remove the standalone solver wrapper module. Its thin shaping — build a
solve-budgeted Session, run card_game::Session::solve(), extract the first
useful move — moves onto the domain type in solitaire_core as
GameState::solve_first_move() / GameState::solve_fresh_deal(), with the budget
consts and the SolveOutcome alias re-exported from solitaire_core.

Solving is deterministic, IO-free game logic, so core (which already owns
GameState and exposes session().solve()) is its correct home; solitaire_data is
the persistence/sync layer and never should have owned it.

Consumers now call the core API directly:
- engine: pending_hint (solve_first_move), game_plugin + play_by_seed_plugin
  (solve_fresh_deal), input_plugin (budget consts)
- assetgen: gen_seeds + gen_difficulty_seeds (solve_fresh_deal)

The solver tests move to solitaire_core. cargo test --workspace and
clippy --workspace --all-targets -- -D warnings both green.

Resolves the "delete the solver" directive — card_game provides the solver.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chore: gitignore local helper scripts f0871c03e8 
Ignore the local token-saving Go helpers under scripts/ (peek, cargoclip,
testfail, diffclip, cratemap, sessionpack, etc.) via scripts/*.go. These are
inspection-only dev tools, not committed. Tracked scripts/*.sh and *.md are
unaffected. Replaces a broad, machine-local .git/info/exclude rule.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-11 22:13:09 +00:00
chore: gitignore .claude-flow scratch dirs d045781119 
A claude-flow tool run left solitaire_engine/src/.claude-flow/. Ignore
.claude-flow/ anywhere in the tree, matching the existing agent-tooling
artifact rules.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 2 commits 2026-06-12 02:37:17 +00:00
refactor: replace local DrawMode with upstream klondike::DrawStockConfig (#82) 5c992cbdca 
DrawMode was a 1:1 mirror of klondike::DrawStockConfig (DrawOne/DrawThree).
Delete it and use the upstream type everywhere; re-export DrawStockConfig from
solitaire_core. config_for assigns draw_stock directly and draw_mode() returns
session.config().inner.draw_stock.

Serde is unchanged — DrawStockConfig serialises to the same "DrawOne"/"DrawThree"
named variants, so persisted game_state.json / replay JSON stay byte-compatible
(no migration). Field/method/variable names containing draw_mode are unchanged.

35 files, mechanical type swap across all crates. Implemented via a multi-agent
workflow (core → per-crate consumers → verify). cargo test --workspace and
clippy --workspace --all-targets -- -D warnings green.

Closes #82

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
refactor: remove card.rs / card_to_id; use card_game::Card directly (#83) e0a858d4e8 
card_to_id was a frankenstein 0..=51 id shim. Replace it with card_game::Card:
- feedback_anim deal jitter now seeds off a hash of the Card itself
- radial_menu RightClickRadialState.cards: Vec<u32> -> Vec<Card>
- wasm CardSnapshot.id: u32 -> Card (serialises transparently as a plain JS
  number, the same opaque key the renderer already used; new test asserts the
  JSON id field is a number)
- wasm DebugInvariantReport deck-completeness check reworked from a [bool;52]
  index into a HashSet<Card> + Card::new reference deck; the out-of-range check
  is dropped since a Card is always valid

Delete card.rs entirely: the Card/Deck/Rank/Suit re-exports move to the crate
root and the 69 `solitaire_core::card::` import paths flatten to `solitaire_core::`.

The JS card.id is purely an opaque identity key (Map key / dataset.cardId, no
arithmetic, card faces render from rank+suit), so the value change is safe.
cargo test --workspace and clippy --workspace --all-targets -- -D warnings green.

Closes #83

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-12 19:45:02 +00:00
refactor: persist replay/save moves as KlondikeInstruction, not pile coords (#89) 9bbb57134f 
Pile-position types (Tableau, Foundation, KlondikePile, KlondikePileStack)
are runtime-only and have no serde upstream. Per Rhys's guidance, the
persistence layer now stores the moves (KlondikeInstruction) rather than
board coordinates, decoding back to runtime pile positions on demand.

Core / data:
- game_state: instruction_history() -> Vec<KlondikeInstruction>; add
  instruction_to_piles() and apply_instruction(); drop AnyInstruction.
- klondike_adapter: delete the entire Saved* serde mirror section
  (SavedTableau/Foundation/SkipCards/KlondikePile/TableauStack/
  KlondikePileStack/DstFoundation/DstTableau/SavedInstruction).
- replay: drop the bespoke ReplayMove serde mirror; Replay.moves is now
  Vec<KlondikeInstruction>; REPLAY_SCHEMA_VERSION 2 -> 3.
- storage: game_state save format v3 rejected (v4/v5 only).

Engine / wasm consumers:
- record via KlondikeInstruction (stock click = RotateStock).
- playback decodes each instruction to (from, to, count) against the
  live state via instruction_to_piles, then fires the canonical event;
  undecodable instructions are skipped with a warning, never panic.
- remove all use solitaire_data::ReplayMove and Saved* imports.

Workspace check, clippy -D warnings, and the full test suite all pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-12 20:07:29 +00:00
chore: add pedantic workspace lints (#90) ceb9c950a1 
Add [workspace.lints.rust] and wire each member crate up with
[lints] workspace = true:

  unsafe_code = "deny"        (forbid would break the Android JNI build)
  single_use_lifetimes = "warn"
  trivial_casts = "warn"
  unused_lifetimes = "warn"
  unused_qualifications = "warn"
  variant_size_differences = "warn"
  unexpected_cfgs = "warn"

unsafe_code is "deny" rather than the issue's "forbid" so the three
Android JNI FFI modules (android_keystore, android_clipboard, safe_area)
can opt back in with a scoped #![allow(unsafe_code)] — forbid cannot be
locally overridden. Pure crates carry no unsafe and stay clean.

Clean up the warnings the new lints surface:
- 150ish unused_qualifications removed via `cargo fix` (purely syntactic
  redundant-path-prefix removals).
- table_plugin: the TABLE_COLOUR import was #[cfg(test)]-gated while the
  camera clear-colour used the fully-qualified path; unqualifying it left
  a non-test build with no import. Made the import unconditional instead.
- assets/sources: the `as &[u8]` casts in embed_*_svg! coerce each
  fixed-size &[u8; N] to a uniform slice so the tuples fit the
  &[(&str, &[u8])] arrays — load-bearing, so scoped #[allow(trivial_casts)].

Workspace clippy -D warnings and the full test suite pass. Android build
not compiled here (needs the NDK; built separately per CLAUDE.md §15) —
the deny + scoped-allow keeps the JNI unsafe blocks legal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-12 21:22:38 +00:00
refactor(core): delegate check_win to Session::is_win 0d5c9cdb1d 
check_win() manually projected through session.state().state().is_win(),
reaching the inner Klondike's is_win. Session::is_win() (card_game 0.4.1)
wraps that exact same projection, so collapse the three-hop reach into a
single-hop delegation. check_auto_complete() keeps its projection because
is_win_trivial() is a Klondike-only method with no Session wrapper.

Behavior is bit-for-bit identical; the test-support override in
is_won()/is_auto_completable() (which call check_win) is untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 2 commits 2026-06-22 18:04:34 +00:00
refactor(core): move compute_time_bonus into scoring module 8995a8ae9c 
The win-time bonus is Ferrous house-rule scoring policy, not a bridge to
the upstream klondike crate, so it does not belong in klondike_adapter.
Relocate it to a dedicated solitaire_core::scoring module and update the
sole caller (win_summary_plugin) and the adapter/settings doc references.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs: correct font-embed claims in font_plugin and CLAUDE.md 6a9352cde1 
font_plugin's module doc claimed a parse failure aborts the program, but
the code warns and continues with glyph-less UI; fix the doc to match.
CLAUDE.md §4.2 listed only audio and the card theme as embedded while
saying "do not embed user fonts"; document that the bundled FiraMono face
is legitimately embedded via include_bytes! as the canonical UI font.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-22 18:33:28 +00:00
refactor(android): forbid unsafe workspace-wide, quarantine JNI in app (#91) 9299176b2d 
Addresses #91: the #90 posture (`deny(unsafe_code)` + three scattered
`#![allow(unsafe_code)]` across solitaire_data and solitaire_engine) punched
unsafe holes into otherwise-pure logic crates. Replace it by *reducing* the
unsafe rather than relocating it, then forbidding it everywhere it can be
forbidden.

Changes:
- solitaire_data: new safe `android_jni` bridge owning the cached `JavaVM`
  and activity `GlobalRef`; exposes `with_env` / `with_activity_env` so
  keystore/clipboard/safe-area never touch a raw handle.
- keystore: drop the `JavaVM::from_raw` init path (now in the app) and
  replace the three `unsafe { JByteArray::from_raw(x.into_raw()) }` casts
  with the safe `JByteArray::from(JObject)` conversion jni 0.21 provides.
- engine clipboard + safe_area: route through the bridge; remove their
  `#![allow(unsafe_code)]` and all `from_raw` calls.
- solitaire_app: becomes the single owner of FFI unsafe. `android_main`
  reconstructs the raw `JavaVM` / activity once (it must, as the cdylib that
  exports `#[unsafe(no_mangle)]`) and registers the safe wrappers. It opts to
  its own `deny`-level lints with two scoped `#[allow(unsafe_code)]`.
- workspace: `unsafe_code` is now `forbid`. Every crate except the app entry
  point is fully unsafe-free.

Net: 7 unsafe sites across three crates collapse to 3 at the OS boundary in
one crate. Verified with host `clippy --workspace -- -D warnings` and an
`aarch64-linux-android` clippy build of solitaire_app (transitively engine +
data); also fixed two latent android-only `collapsible_if` warnings surfaced
in the keystore by the cross-target check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 added 1 commit 2026-06-22 18:39:23 +00:00
style(engine): clear latent android-only clippy warnings in hud_plugin e3b8a403ef 
These six warnings only surfaced on an `aarch64-linux-android` clippy build;
the host workspace gate never compiles the `cfg(target_os = "android")` HUD
tap-toggle code, so they had accumulated unseen.

- unqualify `Vec2` / `TouchInput` (reachable via the bevy prelude) in the
  android tap tracker, plugin wiring, and `toggle_hud_on_tap` signature
- make the `PausedResource` import unconditional and unqualify its use in
  `handle_hint_button` (host-compiled, so the import had been android-gated
  purely to satisfy `toggle_hud_on_tap`)
- allow `clippy::too_many_arguments` on the `toggle_hud_on_tap` Bevy system
- collapse a nested `if let` / `if` into a let-chain

Verified clean on both host `clippy -p solitaire_engine --all-targets
-- -D warnings` and `aarch64-linux-android` clippy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
funman300 merged commit dde65a7e30 into master 2026-06-22 18:44:38 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
android
bug
correctness
enhancement
performance
security
server
test
ui
ux
wasm
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
funman300 (Alex)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: funman300/Ferrous-Solitaire#88
Delete Branch "refactor/strip-card_game-redundancies"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?