fix: id, description of concrete floor without roof #3891
Conversation
would it be compatible with old saves or maps that use the id |
|
no |
|
@chaosvolt would it be possible to define migration for terrain ids? |
|
Maybe add "t_thconc_floor_nofloor" to uncategorized.json in obsoletion |
|
I think you can use |
There was a problem hiding this comment.
It's not worth the incompatibilities here since people won't confuse floor_nofloor as not having a floor instead of not having a roof.
You would need to update every mapgen palette in base game to use the new ID or you'll have broken a lot of locations, and that's still not mentioning how many mods use the old ID that would need to scramble to update just to let people run their game again.
There was a problem hiding this comment.
Personally I feel that the ID has fairly low risk of confusing people (the floor has no floor! :D), but I'm sure someone out there might assume it to be some kinda skylight. The question then becomes, does the risk of "someone's failed attempt to add said skylight to their mapgen becoming our problem" outweigh making the fix for it everyone else's problem?
If alias works correctly (we'll need to test it, namely load-test while retaining an unchanged use of the old terrain somewhere in the JSON, then test further by spawning some of the old terrain in another build without this change then load that save with your JSON changes), then we have the best of both worlds (we can fix the ID without breaking shit for other people). If not, then I'd say retain the old ID instead because it's really not worth it.
| }, | ||
| { | ||
| "type": "terrain", | ||
| "id": "t_thconc_floor_no_roof", |
There was a problem hiding this comment.
| "id": "t_thconc_floor_no_roof", | |
| "id": "t_thconc_floor_no_roof", | |
| "alias": "t_thconc_floor_nofloor", |
Should've actually added the relevant code suggestion to the change request proposal hecc
There was a problem hiding this comment.
I loaded a save with the old terrain from a build without changes into a build with changes.
No error message.
There was a problem hiding this comment.
Uh, you sure about that? The map would be trying to load terrain in its save JSON that's been outright deleted.
There was a problem hiding this comment.
No idea...
In my build without change:
When, I load the save in the build with change:
It seems to change "t_thconc_floor_nofloor" to "t_thconc_floor_no_roof".
There was a problem hiding this comment.
Hmm, that's a good sign then? Weird, I would've expected it to need some kinda migration or alias...maybe we can still assign alias just to be safe though?
There was a problem hiding this comment.
This requires further investigation @chaosvolt
There was a problem hiding this comment.
No idea... In my build without change:
When, I load the save in the build with change:
It seems to change "t_thconc_floor_nofloor" to "t_thconc_floor_no_roof".
I made a migration for aftershock furnitures it may be worth testing one here. I sadly didnt test my migration as I figured if it didnt work it wasnt possible.
|
I close, it's not worth changing anything. |
Purpose of change
Rename id "t_thconc_floor_nofloor" of concrete floor without roof for consistency.
Remove the "roof" part of the terrain description.
Describe the solution
Replace id with "t_thconc_floor_no_roof".
Describe alternatives you've considered
None
Testing
Additional context