Skip to content

Refactor maze generation and solvability check#1

Open
JanKrivanek wants to merge 2 commits intomainfrom
dev/jankrivanek/tst-refactor
Open

Refactor maze generation and solvability check#1
JanKrivanek wants to merge 2 commits intomainfrom
dev/jankrivanek/tst-refactor

Conversation

@JanKrivanek
Copy link
Copy Markdown
Contributor

Refactor maze generation to check solvability after carving paths. Update CarvePath method to return a boolean indicating if the exit is reachable.

Refactor maze generation to check solvability after carving paths. Update CarvePath method to return a boolean indicating if the exit is reachable.
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Integrate solvability check into maze path carving

✨ Enhancement

Grey Divider

Walkthroughs

Description
• CarvePath method now returns boolean indicating exit reachability
• Moved path carving after entry/exit setup for proper initialization
• Eliminated separate IsSolvable check by integrating solvability detection into CarvePath
• Early exit detection during DFS traversal improves efficiency
Diagram
flowchart LR
  A["Generate Maze"] --> B["Setup Entry/Exit"]
  B --> C["CarvePath with DFS"]
  C --> D{"Exit Reached?"}
  D -->|Yes| E["Return true"]
  D -->|No| F["Return false"]
  E --> G["Solvable Maze"]
  F --> H["Regenerate"]
Loading

Grey Divider

File Changes

1. src/MazeSolver/Services/MazeGenerator.cs ✨ Enhancement +13/-6

Integrate solvability detection into path carving

• Changed CarvePath return type from void to bool to indicate exit reachability
• Moved CarvePath invocation after entry/exit setup to ensure proper maze initialization
• Added early exit detection in CarvePath that returns true when exit cell is reached
• Replaced IsSolvable(maze) call with direct use of CarvePath return value
• Added return false at end of CarvePath for paths that don't reach exit

src/MazeSolver/Services/MazeGenerator.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Mar 30, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Exit check won't compile🐞 Bug ✓ Correctness
Description
CarvePath compares maze[newPos].Type (a CellType) to maze.Exit (a Position), which is a
compile-time type mismatch and will fail the build.
Code

src/MazeSolver/Services/MazeGenerator.cs[R67-70]

+            if (maze[newPos].Type == maze.Exit)
+            {
+                return true;
+            }
Evidence
Maze.Exit is declared as a Position, while Cell.Type is a CellType enum, so
maze[newPos].Type == maze.Exit is not type-compatible in C#.

src/MazeSolver/Services/MazeGenerator.cs[61-70]
src/MazeSolver/Models/Maze.cs[10-13]
src/MazeSolver/Models/Cell.cs[6-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CarvePath` currently compares a `CellType` (`maze[newPos].Type`) to a `Position` (`maze.Exit`), which will not compile.
### Issue Context
`Maze.Exit` is a `Position`, not a `CellType`. Decide whether the solvability check should compare positions (`newPos == maze.Exit`) or compare cell types (`maze[newPos].Type == CellType.Exit`), but ensure the types match.
### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[61-74]
- src/MazeSolver/Models/Maze.cs[10-13]
- src/MazeSolver/Models/Cell.cs[6-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Bounds check after indexing🐞 Bug ⛯ Reliability
Description
CarvePath indexes maze[newPos] before verifying maze.IsInBounds(newPos), which can throw
IndexOutOfRangeException when DFS considers directions that step outside the grid.
Code

src/MazeSolver/Services/MazeGenerator.cs[R64-74]

           var newY = pos.Y + dy;
           var newPos = new Position(newX, newY);

+            if (maze[newPos].Type == maze.Exit)
+            {
+                return true;
+            }
+            
           // Check if the new position is valid and unvisited
           if (maze.IsInBounds(newPos) && maze[newPos].Type == CellType.Wall)
           {
Evidence
newPos is computed by adding +/-2 deltas and can be out of bounds (e.g., from (1,1) with dx=-2 =>
x=-1). The Maze indexer directly accesses the underlying array without bounds checks, so
maze[newPos] will throw before the later IsInBounds guard runs.

src/MazeSolver/Services/MazeGenerator.cs[61-75]
src/MazeSolver/Models/Maze.cs[36-44]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`maze[newPos]` is accessed before checking `maze.IsInBounds(newPos)`, allowing out-of-bounds positions to crash maze generation.
### Issue Context
DFS explores directions with +/-2 offsets; positions outside the grid are expected and must be filtered before any indexing.
### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[61-75]
- src/MazeSolver/Models/Maze.cs[36-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. CarvePath result not propagated 🐞 Bug ✓ Correctness
Description
Recursive calls to CarvePath ignore the returned boolean, so even if a deeper recursion determines
reachability, the initial call can still return false, causing repeated regeneration in
Generate().
Code

src/MazeSolver/Services/MazeGenerator.cs[R80-84]

               CarvePath(maze, newPos);
           }
       }
+
+        return false;
Evidence
Generate() uses the return value of CarvePath to decide whether to regenerate, but the recursive
call result is discarded (CarvePath(maze, newPos);). The function then falls through to `return
false`, so success from deeper frames is not observable at the top level.

src/MazeSolver/Services/MazeGenerator.cs[38-47]
src/MazeSolver/Services/MazeGenerator.cs[53-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CarvePath` now returns `bool` but recursive invocations discard the return, so the top-level solvability signal is lost.
### Issue Context
`Generate()` depends on `bool isSolvable = CarvePath(...)`. For DFS-style discovery, successful results must be propagated upward (e.g., `if (CarvePath(...)) return true;`).
### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[38-47]
- src/MazeSolver/Services/MazeGenerator.cs[73-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. Exit unreachable by DFS 🐞 Bug ✓ Correctness
Description
With enforced odd dimensions, the exit is placed at y = height - 1 (even), but CarvePath moves
in steps of 2 from y = 1 (odd), so it can never reach the exit position; additionally, pre-marking
(width-2,height-2) as Path prevents carving into the only cell adjacent to the exit.
Code

src/MazeSolver/Services/MazeGenerator.cs[R35-46]

       maze[1, 1].Type = CellType.Path;
       maze[width - 2, height - 2].Type = CellType.Path;

+        // Carve paths using DFS from (1, 1)
+        var startPos = new Position(1, 1);
+        bool isSolvable = CarvePath(maze, startPos);
+
       // Verify the maze is solvable
-        if (!IsSolvable(maze))
+        if (!isSolvable)
       {
           Log.Warning("Generated maze was not solvable, regenerating...");
           return Generate(width, height);
Evidence
Generate() forces odd width/height, making height-1 even, while CarvePath uses only +/-2
moves so it preserves odd parity and cannot visit the exit coordinate. Also, CarvePath only carves
into CellType.Wall cells, but (width-2,height-2) is set to Path before carving, preventing DFS
from creating a connecting corridor to the exit-adjacent cell.

src/MazeSolver/Services/MazeGenerator.cs[20-36]
src/MazeSolver/Services/MazeGenerator.cs[57-60]
src/MazeSolver/Services/MazeGenerator.cs[72-81]
src/MazeSolver/Services/MazeGenerator.cs[96-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The current `CarvePath`-based solvability detection cannot succeed with the current exit placement/move step and the pre-opened `(width-2,height-2)` cell.
### Issue Context
- `Generate()` normalizes dimensions to odd.
- Exit is placed at `(width-2, height-1)`.
- `CarvePath` moves by +/-2, so it only visits odd coordinates starting from `(1,1)`.
- `(width-2,height-2)` is set to `Path` before carving, but the carve only enters `Wall` cells.
### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[20-47]
- src/MazeSolver/Services/MazeGenerator.cs[53-85]
- src/MazeSolver/Services/MazeGenerator.cs[96-128]
### Implementation direction (choose one)
1) Restore BFS verification: keep `CarvePath` as `void` and use `IsSolvable(maze)` after carving; or
2) If keeping `CarvePath` returning `bool`, make the goal a reachable carve-cell (e.g., reaching `(width-2,height-2)`), ensure that cell is not pre-marked as `Path` before carving (or allow carving/connect-to it), and propagate return values up the recursion chain.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread src/MazeSolver/Services/MazeGenerator.cs Outdated
Comment thread src/MazeSolver/Services/MazeGenerator.cs
Comment on lines 80 to +84
CarvePath(maze, newPos);
}
}

return false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

3. Carvepath result not propagated 🐞 Bug ✓ Correctness

Recursive calls to CarvePath ignore the returned boolean, so even if a deeper recursion determines
reachability, the initial call can still return false, causing repeated regeneration in
Generate().
Agent Prompt
### Issue description
`CarvePath` now returns `bool` but recursive invocations discard the return, so the top-level solvability signal is lost.

### Issue Context
`Generate()` depends on `bool isSolvable = CarvePath(...)`. For DFS-style discovery, successful results must be propagated upward (e.g., `if (CarvePath(...)) return true;`).

### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[38-47]
- src/MazeSolver/Services/MazeGenerator.cs[73-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 35 to 46
maze[1, 1].Type = CellType.Path;
maze[width - 2, height - 2].Type = CellType.Path;

// Carve paths using DFS from (1, 1)
var startPos = new Position(1, 1);
bool isSolvable = CarvePath(maze, startPos);

// Verify the maze is solvable
if (!IsSolvable(maze))
if (!isSolvable)
{
Log.Warning("Generated maze was not solvable, regenerating...");
return Generate(width, height);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

4. Exit unreachable by dfs 🐞 Bug ✓ Correctness

With enforced odd dimensions, the exit is placed at y = height - 1 (even), but CarvePath moves
in steps of 2 from y = 1 (odd), so it can never reach the exit position; additionally, pre-marking
(width-2,height-2) as Path prevents carving into the only cell adjacent to the exit.
Agent Prompt
### Issue description
The current `CarvePath`-based solvability detection cannot succeed with the current exit placement/move step and the pre-opened `(width-2,height-2)` cell.

### Issue Context
- `Generate()` normalizes dimensions to odd.
- Exit is placed at `(width-2, height-1)`.
- `CarvePath` moves by +/-2, so it only visits odd coordinates starting from `(1,1)`.
- `(width-2,height-2)` is set to `Path` before carving, but the carve only enters `Wall` cells.

### Fix Focus Areas
- src/MazeSolver/Services/MazeGenerator.cs[20-47]
- src/MazeSolver/Services/MazeGenerator.cs[53-85]
- src/MazeSolver/Services/MazeGenerator.cs[96-128]

### Implementation direction (choose one)
1) Restore BFS verification: keep `CarvePath` as `void` and use `IsSolvable(maze)` after carving; or
2) If keeping `CarvePath` returning `bool`, make the goal a reachable carve-cell (e.g., reaching `(width-2,height-2)`), ensure that cell is not pre-marked as `Path` before carving (or allow carving/connect-to it), and propagate return values up the recursion chain.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant