Repository navigation
tiletypes: remove plants when their tile is painted away - #5918
Conversation
Painting a tile (e.g. clearing ground to open space) left the plant record behind, so trees kept re-sprouting from empty air and then collapsing over whatever was built below. After painting, a plant rooted at the tile is now removed when the new tiletype can no longer host it: grown trees need a tree-material tile, shrubs and saplings need their respective tile shapes, and unsprouted trees need a tile that still exists. Tree records take their tree_info with them so nothing regrows; leftover canopy tiles remain for the user to paint over, same as after 'plant remove'. Fixes DFHack#5838
20faec4 to
830f539
Compare
| // remaining tiles are left as-is for the user to paint over or collapse | ||
| static void removePlant(df::plant *plant) | ||
| { | ||
| vector<df::plant *> *vec = NULL; |
There was a problem hiding this comment.
| vector<df::plant *> *vec = NULL; | |
| vector<df::plant *> *vec = nullptr; |
NULL is non-standard in C++
| } | ||
| if (vec) | ||
| { | ||
| for (size_t i = vec->size(); i-- > 0;) |
There was a problem hiding this comment.
is there a good reason not to use std::erase here?
| auto col = Maps::getBlockColumn((plant->pos.x / 48) * 3, (plant->pos.y / 48) * 3); | ||
| if (col) | ||
| { | ||
| for (size_t i = col->plants.size(); i-- > 0;) |
There was a problem hiding this comment.
also consider std::erase here
while std::erase will not quit once it's found an instance of the item to remove, and will instead scan the entire vector to remove duplicates, this is actually safer: if (for some reason) there is a duplicate, removing all of them will prevent a dangled reference. remember postel's law.
| } | ||
|
|
||
| auto &all = world->plants.all; | ||
| for (size_t i = all.size(); i-- > 0;) |
| for (int16_t y = lo.y; y <= hi.y; y++) | ||
| { | ||
| df::coord pos(x, y, z); | ||
| if (filter(pos, NULL)) |
There was a problem hiding this comment.
| if (filter(pos, NULL)) | |
| if (filter(pos, nullptr)) |
|
All addressed in c20b277:
|
Addresses review feedback: std::erase also guards against duplicate entries leaving dangling references.
c20b277 to
fc261d7
Compare
|
Added in-game regression coverage in 2a7c742: painting a soil wall over a shrub/sapling removes the plant record, an identity repaint preserves it, and painting over a grown tree's trunk removes the tree. Also covers the console/help/error paths. All 6 tests pass in-game. |
Summary
Fixes #5838
Painting a tile (e.g. clearing ground to open space) left the plant record behind, so trees kept re-sprouting from empty air and then collapsing over whatever was built below.
After painting, a plant rooted at the tile is now removed when the new tiletype can no longer host it:
tree_info) need aTREE/ROOT/MUSHROOMmaterial tileSHRUB/SAPLINGshaped tilesDRY_TREE/WET_TREErecords on ground tiles) need a tile that still existsTree records take their
tree_infowith them so nothing regrows; leftover canopy tiles remain for the user to paint over, same as afterplant remove. Removal mirrors theplantplugin's uncat-and-delete flow.Both paint paths are covered: single tiles (
paintTile, also used bygui/tiletypesviatiletypes_setTile) and area brushes (paintArea, which skips filtered-out tiles).Testing
Built and installed locally (win64, DF 53.16), then exercised in a live fort:
TreeTrunkPillarbase to open space viatiletypes_setTile— plant record removed fromworld->plants.all, tile becameOpenSpace.paintAreapath) — the in-area plant was removed while a plant just outside the brush survived untouched.