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

1 ml of gasoline burns for 14 hours #73735

Open
aenodss opened this issue May 13, 2024 · 8 comments
Open

1 ml of gasoline burns for 14 hours #73735

aenodss opened this issue May 13, 2024 · 8 comments
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Help Wanted Not particularly urgent or easy (see Good First Issue for this), but help is appreciated with this! (S2 - Confirmed) Bug that's been confirmed to exist

Comments

@aenodss
Copy link

aenodss commented May 13, 2024

Describe the bug

While trying to learn about fire, I dropped a unit of gasoline and set fire to the tile containing it, it created a raging fire that lasted 14 hours.

Attach save file

N/A

Steps to reproduce

  1. drop a unit of gasoline
  2. set it on fire
  3. wait

Expected behavior

I expected that fire would last reasonably, similar to 0.G (about 7 minutes).

Screenshots

No response

Versions and configuration

  • OS: windows
  • Game version: cdda-experimental-2024-05-12-1705 39e2afa
  • Graphics version: tiles
  • Game language: System language
  • Mods loaded: [[Dark Days Ahead [dda], Disable NPC Needs [no_npc_food], Portal Storms Ignore NPCs [personal_portal_storms], Slowdown Fungal Growth [no_fungal_growth]]

Additional context

No response

@aenodss aenodss added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label May 13, 2024
@XygenSS
Copy link
Contributor

XygenSS commented May 13, 2024

Confirmed

Screenshot_2024-05-13-12-36-06-614_com cleverraven cataclysmdda experimental-edit

  • 1 unit of gasoline dropped on a brazier lasts for 24 hours

  • the same dropped on the pavement instantly develops to a raging fire & seems to last just as long

  • OS: Android

    • OS Version: Manufacturer: Xiaomi; Model: 23046RP50C; Release: 14; Incremental: V816.0.4.0.UMYCNXM;
  • Game Version: cdda-experimental-2024-05-01-1556 53a0a73 [64-bit]

  • Graphics Version: Tiles

  • Game Language: English [en]

  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]

@TealcOneill
Copy link
Contributor

/Confirm

  • OS: Windows
    • OS Version: 10.0.19045.4291 (22H2)
  • Game Version: cdda-experimental-2024-05-12-1705 39e2afa [64-bit]
  • Graphics Version: Tiles
  • Game Language: System language []
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]

@github-actions github-actions bot added (S2 - Confirmed) Bug that's been confirmed to exist and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels May 13, 2024
@RenechCDDA
Copy link
Member

Well, the resulting fire values are being multiplied a random number between 100 and 200. That's accounting for the time_added going from 300.0 to (in my case) 40800.0.

int stack_burnt = rng( type->stack_size / 2, type->stack_size );

I can confirm the behavior is different in 0.G though.

item::simulate_burn hasn't changed since the 0.G branch, so something must have changed elsewhere. I went through the field processor, item::burn, even the firestarter actor... didn't find any obvious cause.

@RenechCDDA
Copy link
Member

RenechCDDA commented May 13, 2024

I've bisected it to probably #73069 (yep, also me!). April 28: no issues, may 1st: issue. That PR was merged on the 29th and touched related code, so it is by far the most likely candidate.

...I am not finding the reason for this change in behavior, though.

@RenechCDDA
Copy link
Member

Something I am not understanding is that the reported field age has become an incredibly large negative number.

image

In versions before this issue appears, the field age is uniformly set to 601 right after lighting the fire.

@ZhilkinSerg
Copy link
Contributor

Maybe some code is adding field with infinite duration?

@AnotherSeawhite
Copy link
Contributor

Could this be related with #73495?

@RenechCDDA
Copy link
Member

RenechCDDA commented May 15, 2024

Okay. This is complicated. You can observe the change in behavior by adding the INCENDIARY ammo_effect back onto gasoline. Removing IGNITE is not sufficient - it must have the INCENDIARY tag. (This little fact tripped me up when initially testing)

Having INCENDIARY ammo_effect at all causes the gasoline ammo to cookoff.

obj.ammo->cookoff = ammo_effects.count( "INCENDIARY" ) > 0 ||
ammo_effects.count( "COOKOFF" ) > 0;

Because it cooks off, it returns true in item::will_explode_in_fire(), causing the item to be erased when the fire field processes it (the processor assumes it exploded, regardless of whether or not it did) and does not pass it on to be considered a 'fuel'.

it = items_here.erase( it );

for( auto fuel = items_here.begin(); fuel != items_here.end() && consumed < max_consume; ) {

Because it is not considered a fuel, it does not simulate burn. Note that item::burn is passed fire_data as a reference, and it modifies that fire_data directly.

bool destroyed = fuel->burn( frd );

But now that is not INCENDIARY and not "blowing up", it does get passed into item::simulate_burn() which has that previously mentioned behavior with charges. Also item::burn() itself is setting a huge negative number of charges (which itself should not be possible). There are many, many parts here that are working counterintuitively.

TL;DR: the game previously assumed it "blew up" and discounted it as fuel.

The simplest way to resolve this is probably to force IGNITE ammos to cookoff (diff below). This will restore the old behavior, but does not actually change the calculations which were giving crazy results. It merely goes back to skipping those calculations.

index 7c883c125b..3912921216 100644
--- a/src/item_factory.cpp
+++ b/src/item_factory.cpp
@@ -405,6 +405,7 @@ void Item_factory::finalize_pre( itype &obj )
             mats.find( material_oil ) == mats.end() ) {
             const auto &ammo_effects = obj.ammo->ammo_effects;
             obj.ammo->cookoff = ammo_effects.count( "INCENDIARY" ) > 0 ||
+                                ammo_effects.count( "IGNITE" ) > 0 ||
                                 ammo_effects.count( "COOKOFF" ) > 0;
             static const std::set<std::string> special_cookoff_tags = {{
                     "SPECIAL_COOKOFF"

@RenechCDDA RenechCDDA added [C++] Changes (can be) made in C++. Previously named `Code` Help Wanted Not particularly urgent or easy (see Good First Issue for this), but help is appreciated with this! labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Help Wanted Not particularly urgent or easy (see Good First Issue for this), but help is appreciated with this! (S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

No branches or pull requests

6 participants