Skip to content

fix(walls): keep doors and pieces when walls connect - #216

Open
sasj93 wants to merge 2 commits into
opentibiabr:mainfrom
sasj93:sasj93/wall-connection-fixes
Open

sasj93 wants to merge 2 commits into
opentibiabr:mainfrom
sasj93:sasj93/wall-connection-fixes

Conversation

@sasj93

@sasj93 sasj93 commented Nov 1, 2026 •

Copy link
Copy Markdown

Summary

Fixes bugs in how walls connect when they are drawn: doors lost their kind, walls could disappear, and one loop read past the end of an array.

Fixes

  • Out-of-bounds read. The alternate wall loop in WallBrush::draw started at alignment + 1 and only stopped when it came back to alignment, so it walked past the end of wall_items. It now wraps around the 16 shapes.
  • Doors keep their kind and attributes. When doWalls changes the shape of a door, it looks for the same door type in the new shape, so a locked door stays locked. The action, unique and door ids carry over.
  • Walls no longer disappear. When a brush has no item for a shape, the current wall stays instead of being deleted.
  • Iterator skip removed. An extra ++it before a break could skip the next item on the tile.
  • getDoorTypeFromID follows redirects. It now checks brushes linked with redirect and stays within the bounds of the array.
  • Brushes without a pole start straight. The first shape after the pole in the table is a corner, so every railing without a pole started with an L. These brushes now start with a straight piece, and their ends continue the run.

How to test

  1. Draw a wall and put a locked door on it, then draw another wall next to it. The door stays locked and keeps its ids.
  2. Draw a line with moist railing1, which has no pole. Both ends are straight and there is no L.
  3. Draw a closed rectangle with moist railing1. The bottom right corner uses the corner piece.
  4. Draw stone wall as a rectangle. Nothing changes for brushes with a pole.

Documentation

No documentation change.

Tests / Validation

  • Built alone from this branch on Windows with the windows-release preset.
  • clang-format 17 passes on the changed files. cppcheck reports nothing on wall_brush.cpp.
  • The door and shape handling was simulated on house layouts of the global map of canary, and the fix for railings was checked against which pieces that map uses at each junction.

Not done: the steps above were not run by hand on the final commit, and nothing was run on Linux or macOS.

Related

#220 also changes source/wall_brush.{h,cpp}, because it needs getDoorItemID too. Whichever merges second needs a small conflict resolution, and I will rebase it.

🤖 Generated with Claude Code

Connecting walls could replace a door with a plain wall, delete a wall with no piece
for its shape, and read past the end of the alignment table. Brushes without a pole
started with their corner piece, which drew an L at the start of every railing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7380829d-4aaf-48fd-bf27-3be291796811
Commits

Reviewing files that changed from the base of the PR and between 06894f5 and 4f6905f.

Files selected for processing: 2

  • source/wall_brush.cpp
  • source/wall_brush.h
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

majestyotbr

This comment was marked as resolved.

@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

@dudantas dudantas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please keep the existing door when both shape passes lack an equivalent door.

Comment thread source/wall_brush.cpp
}
}

while (true) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When the second shape also lacks a matching door, execution falls through to the ordinary wall_items pool and can replace the door with a wall. The shipped stone-wall data makes this reachable: isolated closed horizontal door 6251 can select ordinary pole 1296. Route that case to the existing id == 0 preservation path so the door keeps its item, action and unique IDs.

Suggested change
while (true) {
while (!wall->isBrushDoor()) {

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.

3 participants