skara-brae-common/docs/openup/analysis-design/design-wi-1-injectable-rng.md

WI-1 Design — Injectable RNG (skara-brae-server#1, PSP1)

Revision 2 — after 1B Design Review (defects D1–D5, D7; D1 blocker: ThreadRng !Send across .await).

Requirements (source: Forgejo issue #1)

1. No direct rand::rng() in non-test code outside a single construction point.
2. create_character() and resolve_combat_full() accept an injectable RNG.
3. All existing tests green; cargo fmt --check, clippy --all-targets -D warnings, cargo test clean.

Out of scope: route-handler refactor, typed responses, magic numbers.

Note: resolve_combat_full already accepts rng: &mut impl Rng (combat.rs:174–183) — only the call-site argument changes. The issue's phrasing overstates; its train_stat() precedent reference is also inaccurate (train_stat takes no RNG). Actual precedents: pick_enemy, roll_stats, resolve_attack, open_ended_roll.

Design

Pattern: domain functions take rng: &mut impl Rng; request handlers obtain their RNG from a single shared constructor in sb-api:

fn request_rng() -> StdRng { StdRng::from_os_rng() }

StdRng (not rand::rng()/ThreadRng) is required because the handler futures must stay Send for axum: a ThreadRng binding held across the handlers' .await points makes the future !Send and fails to compile (D1/D2). StdRng is Send + Sync and matches the seeded StdRng used in domain tests.

Changes:

1. sb-core/src/character.rs — create_character Signature: create_character(rng: &mut impl Rng, user_id: u32, name: String, race: Race, class: Class) -> Character. Drops the internal let mut rng = rand::rng();, forwards rng to roll_stats (which already takes &mut impl Rng). No other body change.

2. sb-api/src/routes/mod.rs — request_rng() helper + create-character handler (~line 288) Add request_rng() (single construction point in the codebase, satisfying the acceptance criterion literally). Create handler: let mut rng = request_rng(); then create_character(&mut rng, ...).

3. sb-api/src/routes/mod.rs — run_adventure handler Replace the three rand::rng() constructions (lines 442, 497, 504) with one let mut rng = request_rng(); before the first guarded block; thread &mut rng into domain::pick_enemy, combat::resolve_combat_full (replacing the inline &mut rand::rng() argument), and the loot-roll rng.random_bool(...) loop. Hoist the block-scoped use rand::Rng; (line 503) so random_bool resolves at the consolidated call sites. Being Send, the rng may cross .await points legally.

4. sb-core/src/character.rs — new test (new object, 18 LOC planned) create_character_is_deterministic_under_seeded_rng: two StdRng::seed_from_u64(42) runs produce identical characters compared via format!("{:?}") (Character derives no PartialEq — D5); covers stats/HP/energy/nerve/spells and id/gold. No different-seed inequality assert (probabilistic flake, review nit — dropped).

Coverage & exclusions (acceptance audit)

All four non-test rand::rng() sites covered (character.rs:130; routes/mod.rs:442, 497, 504). Workspace audit found exactly one other direct randomness construction: sb-auth/src/hash.rs:8 uses getrandom::fill for password-salt entropy — infrastructure entropy, not domain randomness; deliberately excluded (out of scope).

2B Code Review record (rev 2.1)

Implementation deviations from the letter of rev 2 (both accepted, recorded per review nit D11):

• use rand::Rng; remains block-scoped in the loot section (its only use site; matches file style) instead of hoisted.
• The create-handler rng is wrapped in a scoping block (let mut char = { let mut rng = request_rng(); ... }) instead of a plain let mut rng = request_rng(); — tighter drop scope before the following awaits.

Fmt findings D9/D10 fixed on the touched lines; repo-wide pre-existing fmt debt (HEAD fails cargo fmt --check unrelated to this WI) was NOT touched — no drive-by reformat.

Alternatives considered

• AppState-held shared RNG: rejected — single-writer contention, no benefit.
• ThreadRng with tight scoping/drop before awaits: rejected — fragile (lexical drop scopes + await regions), StdRng construction is equivalent and robust (D1 correction).
• Generic R: Rng type parameter instead of impl Trait: equivalent; &mut impl Rng matches the existing services.rs style.

Size plan (from PROBE, method C)

Part	B	D	M	A
character.rs	316	0	3	0
routes/mod.rs	1049	2	12	2 + 4 (request_rng, review rework)
new test	—	—	—	18

Plan N = 35 (original) → 39 after review rework; predicted N = 42 (70% PI 37–73) — rework keeps the plan inside the interval. Time 105 min (75–135).