Skip to content

feat: Champion system architecture - unify data sources, deterministic RNG, and add tests #234

@NicoRuedaA

Description

@NicoRuedaA

Problem or opportunity

The champion system in OLManager has accumulated significant technical debt during the migration from football to MOBA. While it works, there are several architectural issues that create risk for future development:

1. Two sources of truth for the champion catalog

data/lec/draft/champions.json is compiled into the Rust binary twice via include_str! — once in ofm_core::champions::champion_catalog() (for the meta/patch/scouting system) and once in db::game_database::ensure_champions() (for SQLite seeding). Additionally, champion_catalog() caches results in a OnceLock, meaning the catalog exists in three different representations (JSON file, SQLite table, in-memory static). If the JSON schema changes and only one parser is updated, silent bugs appear — this has already happened, requiring hotfix migrations v031 and v032.

2. Non-deterministic RNG in patches and scouting

apply_patch() and process_meta_discovery() call rand::rng() without a seed, even though ChampionPatchState.rng_seed exists and is initialized but never consumed. This means:

  • Save files cannot reproduce the same patches on reload
  • No "same seed, different choices" feature is possible
  • Bugs in specific patches cannot be deterministically reproduced

3. N+1 queries in champion stats

resolve_champion_name() queries SQLite once per champion in matchups, synergies, and top players. For a champion with 20 matchups + 20 synergies, that's roughly 50 individual queries when a JOIN or a simple HashMap cache would suffice.

4. Missing tests for core logic

champions.rs contains 1,313 lines of complex gameplay logic (patch generation with drift + mean reversion, mastery decay, scouting discovery, soloQ calculation, coach delegation) with zero tests. Probabilistic behavior + complex transformations = undetected regressions.

5. Region hardcoded in data paths

data/lec/ is hardcoded throughout. The champion system has no concept of "active region" or "season", making multi-region support harder than it needs to be.

6. Legacy code and empty scraper

champion_training_target (singular legacy field) coexists with champion_training_targets (array) in migration compatibility code. The scraper/ directory exists but contains only node_modules/ with no scripts, meaning champion/player data updates require manual JSON editing.

Proposed solution

Implement a phased architecture overhaul:

Phase 1 — Unify source of truth (P0)

Eliminate the dual source of truth by making champion_catalog() read from SQLite via the existing champion_repo::get_all_champions() instead of the compiled JSON. This removes the OnceLock, the second include_str!, and ensures one canonical representation. The champion seeding pipeline (data/lec/draft/champions.jsonseed_from_json() → SQLite) remains unchanged — only the consumer side is unified.

Phase 2 — Deterministic RNG (P1)

Use the existing ChampionPatchState.rng_seed to seed rand_pcg::Pcg64 in apply_patch() and process_meta_discovery(). This makes patches reproducible on save reload and enables deterministic tests.

Phase 3 — Fix N+1 + de-hardcode region (P2)

Add a name resolution cache (HashMap<String, String>) in champion_stats() and refactor data paths to use a constant/config instead of hardcoded data/lec/.

Phase 4 — Add tests + clean legacy code (P3)

Add property-based tests (invariants) for apply_patch(), mastery_decay(), and soloq_calculation(). Remove the legacy champion_training_target field and its compatibility code. Document/script the scraper pipeline.

UX impact

  • Save system: Deterministic patches mean save files behave consistently on reload. No schema changes required.
  • Data: No user-facing data format changes. Internal architecture only.
  • Documentation: Updated ADRs for champion data flow and seeding pipeline if applicable.

Acceptance criteria

  • Phase 1: champion_catalog() reads from SQLite, OnceLock removed, both include_str! calls consolidated to one
  • Phase 2: apply_patch() and process_meta_discovery() use seedable RNG from ChampionPatchState
  • Phase 3: N+1 queries eliminated in champion_stats(), region path moved to configurable constant
  • Phase 4: Tests for apply_patch() invariants (exactly 4 buffs, 4 nerfs, scores within range), mastery_decay() invariants (never below MIN_MASTERY), soloq_points_for_player() (within [3000, 7000] range)
  • Phase 4: Legacy champion_training_target field removed, scraper pipeline documented or scripted
  • All existing DB tests (123+) continue to pass after each phase

Scope

Gameplay

Approval pre-flight

  • I searched existing issues and did not find a duplicate.
  • I understand maintainers must add status:approved before implementation starts.
  • I will branch from development using type/lowercase-slug if this is approved.
  • I identified whether this changes docs, release behavior, licensing, or data provenance.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions