Skip to content

refactor: use BoardView indexes in Bomberman#30

Merged
patrickleet merged 1 commit into
mainfrom
fix/bomberman-read-model-indexes
May 21, 2026
Merged

refactor: use BoardView indexes in Bomberman#30
patrickleet merged 1 commit into
mainfrom
fix/bomberman-read-model-indexes

Conversation

@patrickleet
Copy link
Copy Markdown
Collaborator

@patrickleet patrickleet commented May 21, 2026

Summary

  • replace Bomberman repository-wide predicate find usage with BoardView-backed aggregate ID loading
  • use BoardView turn/explosion counters for tick saga and explosion IDs
  • track per-player bomb placement counts so bomb IDs stay stable without scanning historical bombs

Verification

  • cargo fmt --check
  • git diff --check
  • cargo test --test bomberman
  • cargo test --all-features --test bomberman

Summary by CodeRabbit

  • Refactor
    • Improved internal game state management and data access patterns for more efficient board state loading and tracking.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b2fe809-ae33-4f21-9e0e-478cf87ed707

📥 Commits

Reviewing files that changed from the base of the PR and between c9614ee and a806f4a.

📒 Files selected for processing (5)
  • tests/bomberman/commands.rs
  • tests/bomberman/domain/player.rs
  • tests/bomberman/main.rs
  • tests/bomberman/sim.rs
  • tests/bomberman/views.rs

📝 Walkthrough

Walkthrough

This PR refactors Bomberman command handling to eliminate prefix-based aggregate searching. New helpers load board state and fetch aggregates by explicit IDs stored in the read model. Command handlers now compute state counters (tick_num, explosion_counter, bombs_placed) directly from read-model fields instead of enumerating aggregates.

Changes

Bomberman command refactoring: read-model indexed access

Layer / File(s) Summary
Read model schema updates
tests/bomberman/views.rs, tests/bomberman/domain/player.rs
BoardView gains explosions_created: u64 with serde default, initialized to 0 in BoardView::new and passed through build_board. Player gains bombs_placed: u64, initialized in join and incremented by place_bomb.
Helper functions for aggregate loading
tests/bomberman/commands.rs
New internal load_board, load_indexed_aggregates, and per-aggregate loaders fetch BoardView and derive in-order Player, Bomb, Explosion collections by IDs stored in the read model, replacing prefix-search patterns.
Tick command refactored
tests/bomberman/commands.rs
Tick removes Find bound, uses helpers to load aggregates, computes tick_num from board.turn + 1, initializes explosion_counter from board.explosions_created instead of enumerating, and passes both to build_board.
Join game command refactored
tests/bomberman/commands.rs
Join game removes Find bound, loads current players/bombs/explosions via helpers, appends new player, and rebuilds BoardView using current_board.turn and current_board.explosions_created before commit.
Move player command refactored
tests/bomberman/commands.rs
Move player removes Find bound, loads other players via helpers, constructs reference slices including the moved player, and rebuilds board with current turn and explosions_created.
Place bomb command refactored
tests/bomberman/commands.rs
Place bomb removes Find bound, computes bomb number from player.bombs_placed + 1, loads current board via helpers, filters bombs to exclude same entity ID, and rebuilds board with current counters.
Test infrastructure updates
tests/bomberman/main.rs, tests/bomberman/sim.rs
Generic bounds in bob_move_to_position, Game, and PlayerSim remove sourced_rust::Find requirement and add Get where needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 No more searching through the prefix maze,
With indexed lookups, the board state stays clear—
Counters track explosions and bombs, always in phase,
Each command finds just what it needs to hold dear! 💣✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactoring objective: replacing predicate-based find patterns with BoardView-indexed aggregate loading, which is the core change across all modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bomberman-read-model-indexes

Comment @coderabbitai help to get the list of available commands and usage tips.

@patrickleet patrickleet merged commit 59ace53 into main May 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant