Let fire spread to tiles with items (#35824) #45969
Conversation
anothersimulacrum
added
[C++]
|
I found the PR that changed fire spreading behavior: #32143. I believe the condition would be a bit more readable in the following way (with lines that caused the bug removed): if( can_spread && rng( 1, 100 ) < spread_chance &&
( in_pit == ( dster.id.id() == t_pit ) ) &&
(
( power >= 2 && ( ter_furn_has_flag( dster, dsfrn, TFLAG_FLAMMABLE ) && one_in( 2 ) ) ) ||
( power >= 2 && ( ter_furn_has_flag( dster, dsfrn, TFLAG_FLAMMABLE_ASH ) && one_in( 2 ) ) ) ||
( power >= 3 && ( ter_furn_has_flag( dster, dsfrn, TFLAG_FLAMMABLE_HARD ) && one_in( 5 ) ) ) ||
nearwebfld ||
( one_in( 5 ) && dst.get_item_count() > 0 &&
flammable_items_at( p + eight_horizontal_neighbors[i] ) )
) ) {
... |
|
I am confused. How could the fire spread through walls before? At least at the moment it only checks the 8 neighboring tiles and if they were non flammable/non webbed walls and didn't have items (which they couldn't have) they wouldn't burn. Or were more tiles allowed as spread targets before? Let me check whether removing this line will reintroduce the behavior shown in the PR. It shouldn't as far as I understand the code. If it still works I will update the PR and remove The change would also allow webs to catch fire again without having anything flammable (at a later point we might also want to look into not checking for webs but rather FLAMMABLE flag on field_types). |
|
@satheon49 , sorry, in the code above I also sneakily removed this line: I think this line could be safely removed (it's effect on fire spreading chance is negligible) and, along with removing |
|
Thanks for the explanation but I think it would prevent forest fires (I guess smokey bear approves) since it would not allow fire to slowly crawl over open ground. I would rather add a check for WALL: Flammable walls will already spread fire anyway similar to fungus so this change would just prevent non-flammable walls from spreading fire. |
That's an interesting point. However, I think that this change would be adding new behavior to the fire spreading. Note, previously, because of the I'm not against that change, but I think that it would be better done as a separate PR (as in my opinion it's more invasive and requires separate discussion). |
…preading to empty squares
|
Just a note of confirmation that it works If you're willing, it'd be a good idea to encode this in a test case, but that is far from something that you need to do here. |
|
Maybe once we decide to make further changes to fire like
Though to be totally honest I am not exactly keen on writing them (but that might change if I can teach my IDE to run single tests and be able to jump to failing tests like I am used to from non C++ projects). |
"Unhardcoding" things is always welcome.
I think this is a good idea, but it would require rigorous testing, as it could change the fire behavior in unexpected ways. Also, thank you for contributing. If I could be of any help with anything, feel free to ping me here or on the discord. |
|
I'm not particularly interested in solving the forest fire use case right now, forest fires propagate via completely different mechanisms compared to small campfire like things like we're discussing, and attempting to force "out of control campfires" to be able to jump multiple meters is more likely to cause more local problems. In short, I think the "fires can exist on a tile that has no fuel" feature is actively counterproductive to making fires work correctly. Forest fires primarily propagate via burning underbrush, which our system handles just fine, but also by irradiance once the fire becomes large enough. Essentially a fully engulfed tree is putting out enough IR to dry out and then trigger fires in adjacent trees. We could certainly look into an effect like that, but random single-tile fires shouldn't be triggering that kind of check because it's super expensive. |
I just pointed out that this PR will actually remove a feature that existed in the past. The timeline was like that:
I am not even sure I want forest fires because they are unlikely to ever stop once they get going. |
|
To be clear, in the current state this PR fixes fire spreading to the tiles with flammable items. It doesn't add nor remove fire spreading to the empty tiles and walls (but it does remove code remnants of that from before #32143 ). I think this is ready and good to be merged. |
|
Tested against a pile of "one of every item in the game". |
Summary
SUMMARY: Bugfixes "Allow fire to spread to adjecent tiles with items"
Purpose of change
Fixes #35824
Fire could not spread to adjacent tiles if those tiles were neither flammable nor contained flammable furniture.
Describe the solution
The problem was that either flammable terrain or flammable furniture had to be true for fire to be allowed to spread.
The solution was to check for either flammable terrain, flammable furniture OR flammable items.
Describe alternatives you've considered
Checking terrain, furniture, items OR nearwebfld. Currently there needs to be something stuck in the web for it to catch fire.
If you have a raging fire it spreads fast like REALLY fast. Maybe the one_in( 5 ) check should be increased a bit. Maybe it is a relic from the time when turns where 6 seconds.
Testing