Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retain hole construction #72927

Merged
merged 4 commits into from
Apr 18, 2024

Conversation

PatrikLundell
Copy link
Contributor

@PatrikLundell PatrikLundell commented Apr 9, 2024

Summary

None

Purpose of change

Fix #72758, i.e. the closing over of a constructed hole into a roofed room below on a save/load cycle.

Describe the solution

Add code on hole construction to check what the terrain below is and transform it into the corresponding terrain resulting from smashing the roof if the terrain below is a roofed one.

Describe alternatives you've considered

  • Completely new JSON support data. Using the bashing data is probably good enough.
  • Hope someone can tell me how to extract region terrain data in a non stupidly convoluted manner.
  • Bring out the heavy machinery and change map.ter_set() to do whatever is needed when a tile with a roof is added or removed, as well as take care of the removal of a roof and replacing it with open air. This would also require the replacement of many, possibly most, tinymaps with smallmaps (to get access to the Z levels above/below), which could have performance implications. I'm getting more and more convinced that is the direction in which we'll need to go, but it's definitely not going to fit in a 500 line change limit. It might be possible to do gradually by having the ter_set operation ignore the additional Z level stuff if called by a smallmap, and thus rely on the flaky add_roofs code during the transition (after all, that's what's done currently).

Testing

Using a copy of the save from the bug report as a base:

  • Teleport to the burned down evac shelter.
  • Wait until the companion finishes digging.
  • Verify that examining the hole shows a hole with dirt beneath.
  • Save/load to verify the hole isn't closed up.
  • Quit.
  • Delete the save copy and copy the save again.
  • Teleport to the shelter.
  • Go down the stairs and debug change the tile beneath the tile worked on to t_floor_resin.
  • Go back up and wait for the companion to finish.
  • Examine the hole, and verify it just tells you there's a resin floor below (there's no description difference between the two types of floor).
  • Go down the stairs and debug examine the resin tile and verify it's actually t_platform_resin.
  • Save/load to verify the hole isn't closed up.

Additional context

Important notes:

  • The only terrains that supported roof bashing are soil and rock, neither of which actually have roofs.
  • The field for roof bashing is set to whatever ordinary bashing results in. This, unfortunately, is usually t_null.
  • This PR adds roof bashing to t_floor_resin both as a proof of concept and as a means to test the change.
  • The fallback when (roof) bashing data is t_null (almost all cases, unfortunately) is t_rock_floor if below z = -1, and region soil otherwise. There's a need for a significant amount of JSON work to provide JSON data to make bashing yield more reasonable results. This probably includes the definition of roof less versions of roofed terrain.
  • This leads to the reason for the selection of mi-go resin as the example transformation. That is a case where there's already a suitable roof less floor provided, and so a minimum need for additional JSON. If I was to try to do the similar thing for the linoleum floor present (apart from not testing the t_null case), I'd have to define a roof less version of the tile (plus the cascade resulting from it being colored), a construction definition for it (and the others), as well as probably change the construction process to first create the roof less version and then go on to the roofed one (unless the linoleum is an addition to e.g. concrete, in which case there would have to be both a construction on roof less concrete (which I think exists) and roofed concrete, as well as the construction of the roofed version from the roofless one. There's a lot of JSON (and testing) work to implement everything properly...
  • This PR ONLY addresses the case of actively constructing a hole. There are other cases that should benefit from the same kind of treatment:
    • the collapsed tower opening holes in the ground that are closed on save/load.
    • zombies smashing up basements.
    • Explosions exposing structures beneath (haven't seen issues with this myself, but probably haven't seen uncovered structures...).
    • May also be relevant to burning building collapses (haven't seen issues with this myself, I think).

Also, I suffer from the view not being properly updated, i.e. the 3D view visually displays the terrain that was below before the conversion, while looking at it ('x') displays the correct terrain below. I've tried various map cache updates and invalidation calls, but none work. The appropriate call probably is located elsewhere. However, I need help to find where to look for it. It can be noted that the visual display is corrected after a save/load cycle, but isn't corrected by moving away (outside of reality bubble range) and then return.

@PatrikLundell PatrikLundell marked this pull request as draft April 9, 2024 11:37
@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Crafting / Construction / Recipes Includes: Uncrafting / Disassembling [C++] Changes (can be) made in C++. Previously named `Code` Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. labels Apr 9, 2024
@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Apr 9, 2024
@PatrikLundell PatrikLundell marked this pull request as ready for review April 16, 2024 06:52
@Maleclypse Maleclypse merged commit 5b6b22d into CleverRaven:master Apr 18, 2024
39 of 43 checks passed
@PatrikLundell PatrikLundell deleted the retain_hole_construction branch April 18, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Save/load cycle turns open air into the floor/ roof
2 participants