enne2/mice
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Refactor rendering logic in MiceMaze and Rat classes for improved clarity and efficiency

Browse Source
This commit is contained in:
Matteo Benedetto
2026-04-14 12:15:16 +02:00
parent bbafc3bbba
commit 509b3433b8
3 changed files with 455 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
REGENERATE_BACKGROUND_REFACTOR_PLAN.md
+419
View File
@@ -0,0 +1,419 @@
# regenerate_background() Analysis And Refactor Plan
## Scope
This document analyzes `Graphics.regenerate_background()` and proposes a staged refactor plan.
Relevant code paths:
- `engine/graphics.py#L146` `draw_maze()` lazily triggers background generation.
- `engine/graphics.py#L155` `draw_cave_foreground()` consumes cave overlay metadata produced during regeneration.
- `engine/graphics.py#L175` `regenerate_background()` builds the static background texture and cave overlay placements.
- `engine/graphics.py#L311` `add_blood_stain()` confirms that blood is intentionally excluded from the background texture and rendered as a separate overlay.
- `engine/sdl2.py#L98` `create_texture()` composites surface tiles into one SDL texture.
- `engine/sdl2.py#L118` `load_image()` explains why the code keeps both surfaces and textures for the same themed assets.
- `engine/maze.py#L13-L15` define `MAP_EMPTY`, `MAP_WALL`, and `MAP_TUNNEL`.
- `rats.py#L79`, `rats.py#L117-L121`, and `rats.py#L163-L165` show where background state is invalidated.
- `rats.py#L335` shows cave foreground rendering happens after the background draw and before units are drawn.
## What The Method Actually Does
`regenerate_background()` is doing more than the name suggests. It is not only “regenerating a background”; it is handling five separate concerns in one place:
1. It walks the logical map cell by cell.
2. It analyzes neighborhood topology around each wall or tunnel cell.
3. It chooses visual variants, including random grass and flower decoration.
4. It builds cave foreground overlay metadata for later explosion-aware rendering.
5. It commits the accumulated surfaces into a single SDL background texture.
That makes it both a planner and a renderer.
## Current Inputs, Outputs, And Side Effects
### Inputs read from `self`
- `self.map.tiles`, `self.map.width`, `self.map.height`
- `self.cell_size`
- `self.grasses`, `self.grass_textures`
- `self.flowers`, `self.flower_textures`
- `self.edges`, `self.corners`, `self.inner_corners`
- `self.caves`
- `self.render_engine`
### Derived helpers inside the method
- `occupied(x, y)` treats every non-empty cell as occupied, so both walls and tunnels count as solid neighbors for topology decisions.
- `is_tunnel(x, y)` is used only for the flower suppression logic in the bottom-right quadrant.
- `draw(...)` appends background surface tiles.
- `draw_cave(...)` appends cave overlay tuples in the format consumed later by `draw_cave_foreground()`.
- `random_wall()`, `random_wall_texture()`, `random_flower()`, `random_flower_texture()` embed random selection directly in the traversal logic.
### Outputs and side effects
- Resets `self.cave_foreground_tiles`
- Builds a local `texture_tiles` list
- Sets `self.background_texture`
- Does not return a value
This means the method is hard to test in isolation because the real output is split across mutable instance state and SDL object creation.
## Functional Walkthrough
### 1. Initialization
The method creates:
- `texture_tiles`: a list of `(surface, x, y)` tuples for the static background
- `self.cave_foreground_tiles`: a list of `(cell_x, cell_y, direction, surface, x, y)` tuples for overlay rendering
- `half_cell`: used to place quarter-cell tiles at 20 px offsets when `cell_size` is 40
This immediately shows a hidden design choice: one map cell can emit up to four quarter tiles rather than a single full-tile sprite.
### 2. Cell iteration
The outer loop traverses every cell of `self.map.tiles`.
- `MAP_EMPTY`: skipped completely
- `MAP_WALL`: potentially emits several quarter tiles
- `MAP_TUNNEL`: emits cave overlays and sometimes grass filler tiles
### 3. Wall rendering logic
For `MAP_WALL`, the method evaluates the four quadrants independently.
#### Top-left quadrant
If the north-west corner is exposed, it chooses among:
- `inner_corners["WN"]`
- `edges["W"]`
- `edges["N"]`
- `corners["NW"]`
based on whether the north and west neighbors are occupied.
#### Bottom-right quadrant
This is the densest branch. It checks south, east, and south-east occupancy.
- If all three are occupied, it usually draws a random grass tile.
- With a 10% chance, it draws a flower instead, but only if the cell is not near the border and none of the neighboring cells involved are tunnels.
- If only south or east are occupied, it chooses `inner_corners["ES"]`, `edges["E"]`, or `edges["S"]`.
- Otherwise it uses `corners["SE"]`.
This branch mixes topology, decoration policy, border constraints, and tunnel suppression all in one nested block.
#### Top-right quadrant
Mirrors the top-left logic using north and east occupancy:
- `inner_corners["EN"]`
- `edges["E"]`
- `edges["N"]`
- `corners["NE"]`
#### Bottom-left quadrant
Mirrors the same pattern using south and west occupancy:
- `inner_corners["WS"]`
- `edges["W"]`
- `edges["S"]`
- `corners["SW"]`
### 4. Tunnel rendering logic
For `MAP_TUNNEL`, the method checks `above`, `below`, `left`, and `right` occupancy and chooses cave overlay sprites.
Observed behavior:
- If there is no occupied tile above, it always draws a grass filler in the bottom-right quarter and uses the `UP` cave sprite.
- If there is an occupied tile above but not below, it uses the `DOWN` cave sprite.
- If both above and below are occupied and the left side is blocked, it may use a full-quarter wall/flower texture in the cave list.
- If both above and below are occupied and the left side is open, it draws a grass filler plus the `LEFT` cave sprite.
- If above and below are occupied, left is blocked, and right is open, it uses the `RIGHT` cave sprite.
This logic appears tuned to the current level topology and asset set rather than representing a complete, explicit rule system for all tunnel neighbor combinations.
### 5. Commit phase
After traversal, the method calls `render_engine.create_texture(texture_tiles, fill_color=(128, 128, 128))` to compose one static SDL texture for the entire maze background.
This is the correct optimization boundary for the current architecture, but it also means SDL concerns leak directly into the generation logic.
## Why The Method Feels Complex
The complexity is not only “too many lines”. It comes from multiple kinds of coupling.
### 1. Mixed responsibilities
The method mixes:
- map analysis
- rule selection
- random decoration
- cave overlay planning
- final rendering commit
Each of these changes for different reasons, so they should not live in the same function.
### 2. Repeated neighborhood queries
Neighbor checks like `occupied(x, y - 1)` and `occupied(x + 1, y)` are recomputed many times, often inside overlapping branches. That makes the code noisy and increases the chance of introducing asymmetric bugs during edits.
### 3. Hidden representation mismatch
Background composition uses SDL surfaces, while cave overlays use textures. That is why the code has parallel helpers like `random_wall()` and `random_wall_texture()`. The behavior is valid, but the representation split is leaking into every branch.
### 4. Randomness is embedded in rule logic
The function directly calls global `random` during traversal. That makes visual behavior hard to snapshot-test or compare before and after a refactor.
### 5. Side effects are scattered across the class lifecycle
Invalidation is controlled elsewhere in `rats.py`, where the code manually clears:
- `self.background_texture`
- `self.blood_layer_sprites`
- `self.cave_foreground_tiles`
This is correct today, but it creates a fragile contract between game flow code and rendering code.
### 6. Tunnel rules are implicit
The tunnel branch contains nested assumptions that are hard to verify by inspection. It is not obvious whether the logic is exhaustive, map-specific, or intentionally asymmetric.
## Important Invariants To Preserve
Any refactor must keep these behaviors unless you explicitly choose to change them:
1. Blood stains remain outside the static background texture.
2. `draw_cave_foreground()` must still be able to swap cave sprites for explosion sprites at runtime.
3. Quarter-tile placement and offsets must remain visually identical.
4. Random flower placement must preserve the current frequency and tunnel/border exclusions, or the change must be documented as a visual redesign.
5. Theme asset selection must keep using surfaces for background composition and textures for runtime overlays unless the render-engine API changes.
## Refactor Goals
The target should be:
- easier to read
- behaviorally stable
- testable without SDL
- explicit about map-topology rules
- easy to extend with new wall or tunnel tile rules
## Recommended Refactor Direction
The safest path is not a full rewrite. It is a staged extraction toward a pure planning layer.
### Stage 1: Name The Concepts
Extract small private helpers without changing data structures yet.
Suggested helpers:
- `_is_occupied(x, y)`
- `_is_tunnel(x, y)`
- `_make_cell_context(x, y)`
- `_append_background_tile(surface, x, y, texture_tiles)`
- `_append_cave_tile(surface, x, y, direction)`
- `_choose_wall_fill(x, y, allow_flower)`
This alone will remove repeated neighbor reads and make the current logic easier to reason about.
### Stage 2: Introduce A Pure Planning Model
Create lightweight data containers, for example:
```python
from dataclasses import dataclass
@dataclass(frozen=True)
class TilePlacement:
surface: object
x: int
y: int
@dataclass(frozen=True)
class CavePlacement:
cell_x: int
cell_y: int
direction: str | None
sprite: object
x: int
y: int
@dataclass(frozen=True)
class CellContext:
x: int
y: int
cell: int
north: bool
south: bool
east: bool
west: bool
north_west: bool
north_east: bool
south_west: bool
south_east: bool
```
Then split the method into:
- `_build_background_plan()`
- `_plan_wall_cell(context, plan)`
- `_plan_tunnel_cell(context, plan)`
- `_commit_background_plan(plan)`
The important shift is this: planning should produce plain Python data first, and SDL texture creation should happen only in the commit step.
### Stage 3: Replace Nested Branches With Rule Helpers
The wall logic is currently “four quadrants, each with a small rule tree”. Keep that structure, but make it explicit.
Suggested helpers:
- `_plan_wall_nw(context, px, py, plan)`
- `_plan_wall_ne(context, px, py, half_cell, plan)`
- `_plan_wall_sw(context, px, py, half_cell, plan)`
- `_plan_wall_se(context, px, py, half_cell, plan)`
This sounds verbose, but it is much easier to review because each helper owns one visual quadrant and one set of rules.
### Stage 4: Isolate Decoration Policy
The flower rule is currently buried inside the `SE` branch. Extract it into a dedicated function such as:
```python
def _should_place_flower(self, x, y, context) -> bool:
...
```
That function should own:
- the 10% probability
- border exclusions
- tunnel exclusions
This makes visual tuning possible without reopening the topology logic.
### Stage 5: Make Tunnel Rules Explicit
Tunnel behavior needs a named rule function with documented cases.
For example:
- `_classify_tunnel(context) -> TunnelPattern`
- `_plan_tunnel_pattern(pattern, px, py, plan)`
Even if the final logic stays the same, naming the tunnel patterns will expose whether the code is intentionally map-specific or accidentally incomplete.
### Stage 6: Centralize Invalidation
Introduce one method such as:
```python
def invalidate_background(self):
self.background_texture = None
self.cave_foreground_tiles.clear()
```
Then use that method from lifecycle points in `rats.py`.
This reduces the chance of future bugs where one part of the cached rendering state is reset and another is forgotten.
## Suggested Final Shape
The long-term shape can stay inside `Graphics` and still be much cleaner:
```python
def regenerate_background(self):
plan = self._build_background_plan()
self._commit_background_plan(plan)
def _build_background_plan(self):
...
def _plan_wall_cell(self, context, plan):
...
def _plan_tunnel_cell(self, context, plan):
...
def _commit_background_plan(self, plan):
self.cave_foreground_tiles = plan.cave_tiles
self.background_texture = self.render_engine.create_texture(
plan.background_tiles,
fill_color=(128, 128, 128),
)
```
This would preserve the current class boundaries while making the core algorithm testable.
## Test Strategy Before Refactoring
Because the function is visual and randomized, refactoring without a guardrail is risky.
Recommended safety steps:
1. Introduce a seeded RNG path so map generation can be deterministic during tests.
2. Add a small test map fixture that exercises walls, corners, borders, and tunnels.
3. Snapshot the produced tile plan, not the SDL texture object.
4. Verify cave overlay tuples are identical before and after the extraction.
5. Add a smoke test for `draw_cave_foreground()` with an explosion unit to ensure cave sprite replacement still works.
## Proposed Implementation Order
### Phase 0: Freeze Current Behavior
- Add a deterministic RNG entry point or injectable random source.
- Capture the current background plan for one or two representative maps.
### Phase 1: Extract Context And Emit Helpers
- Remove repeated `occupied(...)` calls.
- Keep current tuple outputs and current SDL commit behavior.
### Phase 2: Split Wall And Tunnel Planning
- Move wall rules into quadrant helpers.
- Move tunnel rules into a dedicated planner.
### Phase 3: Introduce A `BackgroundPlan`
- Return plain data from planning.
- Keep SDL texture creation in one place.
### Phase 4: Centralize Cache Invalidation
- Replace direct state resets with a single background invalidation method.
### Phase 5: Optional Optimization Pass
- Consider caching immutable plans by `(level_index, theme_index)` if needed.
- Consider precomputing per-cell contexts if profiling shows the planner is still hot.
## Refactor Risks And Questions
These should be clarified before implementation:
1. Are tunnel patterns guaranteed by the level data, or should the code become exhaustive for arbitrary maps?
2. Is `occupied()` intentionally treating tunnels as “solid” for wall topology, or is that only a rendering shortcut?
3. Is the flower placement rule part of the visual identity, or can it be simplified?
4. Do we want to keep both surfaces and textures in the theme cache, or would a render-engine API change be acceptable later?
## Recommended First Refactor PR
The lowest-risk first PR would do only this:
1. Extract `CellContext` creation.
2. Extract the four wall-quadrant planners.
3. Extract tunnel planning into one helper.
4. Leave the tuple formats and SDL commit step unchanged.
That PR would reduce complexity sharply while keeping the visual output almost certainly identical.
## Summary
`regenerate_background()` is complex because it is simultaneously a topology analyzer, decoration policy engine, cave overlay planner, and SDL background composer. The safest refactor is to separate planning from rendering, then isolate wall rules, tunnel rules, and decoration policy into named helpers with deterministic test coverage.
rats.py
+4 -4
View File
@@ -331,14 +331,14 @@ class MiceMaze(
self.unit_positions.setdefault(unit.position, []).append(unit)
self.unit_positions_before.setdefault(unit.position_before, []).append(unit)
self.draw_cave_foreground()
self.render_engine.draw_pointer(self.pointer[0] * self.cell_size, self.pointer[1] * self.cell_size)
# Fourth pass: check collisions and draw
for unit in self.units.copy().values():
unit.collisions()
unit.draw()
self.draw_cave_foreground()
self.render_engine.draw_pointer(self.pointer[0] * self.cell_size, self.pointer[1] * self.cell_size)
self.render_engine.update_status(f"Mice: {self.count_rats()} - Points: {self.points}")
units/rat.py
+32 -56
View File
@@ -205,76 +205,52 @@ class Rat(Unit):
if direction is None:
return False
# Convert the sprite center to tunnel-local coordinates. The visible slice
# is derived from how far that center has progressed across the tunnel cell.
local_x = center_x - cell_x * self.game.cell_size
local_y = center_y - cell_y * self.game.cell_size
half_cell = self.game.cell_size / 2
cell_size = self.game.cell_size
def draw_slice(source_x, source_y, width, height, draw_x=None, draw_y=None):
if width <= 0 or height <= 0:
return False
self.game.render_engine.draw_image(
self.render_x if draw_x is None else draw_x,
self.render_y if draw_y is None else draw_y,
image,
anchor="nw",
tag="unit",
source_rect=(source_x, source_y, width, height),
dest_size=(width, height),
)
return True
if direction == "UP":
visible_ratio = max(0.0, min(1.0, (half_cell - local_y) / half_cell))
visible_ratio = max(0.0, min(1.0, (cell_size - local_y) / cell_size))
visible_height = int(round(image_size[1] * visible_ratio))
if visible_height <= 0:
return False
self.game.render_engine.draw_image(
self.render_x,
self.render_y,
image,
anchor="nw",
tag="unit",
source_rect=(0, 0, image_size[0], visible_height),
dest_size=(image_size[0], visible_height),
)
return True
return draw_slice(0, 0, image_size[0], visible_height)
if direction == "DOWN":
visible_ratio = max(0.0, min(1.0, (local_y - half_cell) / half_cell))
visible_ratio = max(0.0, min(1.0, (local_y - cell_size) / cell_size))
visible_height = int(round(image_size[1] * visible_ratio))
if visible_height <= 0:
return False
source_y = image_size[1] - visible_height
draw_y = self.render_y + source_y
self.game.render_engine.draw_image(
self.render_x,
draw_y,
image,
anchor="nw",
tag="unit",
source_rect=(0, source_y, image_size[0], visible_height),
dest_size=(image_size[0], visible_height),
)
return True
return draw_slice(0, source_y, image_size[0], visible_height, draw_y=draw_y)
if direction == "LEFT":
visible_ratio = max(0.0, min(1.0, (half_cell - local_x) / half_cell))
visible_ratio = max(0.0, min(1.0, (cell_size - local_x) / cell_size))
visible_width = int(round(image_size[0] * visible_ratio))
if visible_width <= 0:
return False
self.game.render_engine.draw_image(
self.render_x,
self.render_y,
image,
anchor="nw",
tag="unit",
source_rect=(0, 0, visible_width, image_size[1]),
dest_size=(visible_width, image_size[1]),
)
return True
return draw_slice(0, 0, visible_width, image_size[1])
visible_ratio = max(0.0, min(1.0, (local_x - half_cell) / half_cell))
visible_width = int(round(image_size[0] * visible_ratio))
if visible_width <= 0:
return False
source_x = image_size[0] - visible_width
draw_x = self.render_x + source_x
self.game.render_engine.draw_image(
draw_x,
self.render_y,
image,
anchor="nw",
tag="unit",
source_rect=(source_x, 0, visible_width, image_size[1]),
dest_size=(visible_width, image_size[1]),
)
return True
if direction == "RIGHT":
visible_ratio = max(0.0, min(1.0, (local_x - cell_size) / cell_size))
visible_width = int(round(image_size[0] * visible_ratio))
source_x = image_size[0] - visible_width
draw_x = self.render_x + source_x
return draw_slice(source_x, 0, visible_width, image_size[1], draw_x=draw_x)
return False
def _get_tunnel_entrance_direction(self, cell_x, cell_y):
directions = [