CleverRaven / Cataclysm-DDA

Cataclysm - Dark Days Ahead. A turn-based survival game set in a post-apocalyptic world.
http://cataclysmdda.org
Other
10.33k stars 4.14k forks source link

Smashing railings on firetower causes floors to be built above #64991

Open natsirt721 opened 1 year ago

natsirt721 commented 1 year ago

Describe the bug

Railings on fire towers do not have a roof above them. When the railing on the fire lookout tower is destroyed, it is replaced with a floor tile that adds a roof. When the terrain is loaded for the first time after smashing the railing, a floor is created one z level up.

Attach save file

N/A

Steps to reproduce

  1. Find fire lookout tower
  2. Go to the top room, smash railings
  3. Observe railings converted to floor on the current z-level (as expected)
  4. Go up one Z level, observe thin air above smashed railings (as expected)
  5. Save and load game
  6. Observe floors appearing above the smashed railings

Expected behavior

Floors don't magically appear when I break railings

Screenshots

No response

Versions and configuration

Additional context

I suspect this is because the fill_ter for that level of the firetower is t_floor instead of t_floor_noroof. So when the terrain gets smashed it is replaced with this. Or maybe railings are hardcoded to be replaced with t_floor when smashed? Interestingly the change is not apparent on the next z-level up until a save-load cycle occurs, but if you try to e.g. place a solar panel on the new floor it won't generate power.

While it would solve this issue, IDK if changing the fill_ter to t_floor_noroof would be ideal, since there is other furniture in the tower that should be roofed over. Ideally there would be some way to specify a 'terrain beneath' for each thing - maybe there is already but I haven't seen any way to do that in the docs. I'm going to keep an eye out for other buildings that have roofed-over furniture but open-air railings and see if they have any tricks that can be used to fix this.

PatrikLundell commented 1 year ago

If I recall correctly, each terrain type that can be smashed has an entry for what it converts into, so it would be railing being defined as turning into t_floor when smashed. I don't think there's any more logic than that, so there's no way to let terrain convert into something depending on other circumstances (e.g. ground at the ground level and floor without a ceiling higher up in a building, or maybe empty air). If railing doesn't generate a ceiling on construction then a smashed railing shouldn't either, logically.

NetSysFire commented 1 year ago

Exactly what happens here. Railings like that should potentially be furniture.

https://github.com/CleverRaven/Cataclysm-DDA/blob/c1158cab31db266ff7e6c03a164d8606e85c66c3/data/json/furniture_and_terrain/terrain-fences-gates.json#L1158

PatrikLundell commented 1 year ago

I agree with @NetSysFire's suggestion, provided there is (or would be) a way to ensure the furniture is immovable so it can't be pushed around.

NetSysFire commented 1 year ago

Shrubs, flowers and such are immovable furniture too. Though I am unsure what flag is used to make them as such, could be either FLOWER or SHRUB. But e.g the topiaries can not be moved either and they lack the flag, instead outputting "You can not grab the abstract topiary". In any case, yes it is possible and easy to implement railings as furniture.

NetSysFire commented 1 year ago

I did a quick source dive (and found a unrelated tiny issue to fix):

https://github.com/CleverRaven/Cataclysm-DDA/blob/f9dd628981ecf5ec0ce98f8b7b14563eec4c5710/src/mapdata.cpp#L1589-L1592

The thing you need is (which also all of the plants have):

"required_str": -1,
natsirt721 commented 1 year ago

Are there other instances of smashable terrain that should get this treatment as well? It seems like fences and other things could be in a similar situation (#48452), and converting them to immovable furniture feels like kind of a kludge. What about something like allowing the OMT file to specify 'terrain under' for things like this? Then you can layer stuff like flowers and grass, or fences on dirt/grass/rocks/gravel etc. And it gets rid of that annoying grab message.

PatrikLundell commented 1 year ago

Unfortunately the data structures allow for exactly one terrain and one furniture (with f_null being an option). In addition, the data usage causes the ceiling to be defined by the terrain on the floor level, rather than any potential ceiling being defined by the floor on the level above.

NetSysFire commented 1 year ago

Are there other instances of smashable terrain that should get this treatment as well?

Possibly. One would need to go through the terrain definitions and check each and every entry manually. Fixing the railing issue alone might be pretty tedious on its own already.

natsirt721 commented 1 year ago

If it isn't too egregious, I'm going to investigate adding a field to the mapgen json that allows the override of ter_set for terrain in them.