Skip to content

/Cataclysm-DDA

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

Rebalanced Fire v2 #23699

Closed
wants to merge 21 commits into from

Conversation

Reviewers

@BevapDin BevapDin

Assignees
No one assigned
Labels
(S5 - OnHold / Stalled) Fields / Furniture / Terrain / Traps Game: Balance
Projects
None yet
Milestone
No milestone
5 participants
@NotFuji @Night-Pryanik @Leland @ZhilkinSerg @BevapDin
@NotFuji
Copy link
Contributor

commented May 9, 2018 •
edited

Because I accidentally deleted the other one.

Anyway...

  • Fire now progresses in 4 stages, the smallest represents a candle flame or lighter, other three are the same as the existing three. To compensate, fire based weapons start stage 2 fires rather than stage 1. Materials are adjusted appropriately, and some outright won't burn unless put in a fire of stage 2+.

  • Start a fire quickly action requires tinder to make a sustainable fire on most materials. Tinder can either be small flammable items on the tile you're trying to light, or 25 charges of the "tinder" item, which is automatically consumed from the player's inventory if they possess enough. The player can still start small fires without tinder if they choose to, but as the fire will likely die out it's probably a bad idea to do so.
    See item::is_tinder() for more information on what does or doesn't count as tinder.

  • Move cost of Start a fire quickly action is determined by the highest fuel rating of the flammable items on a tile. With a "lighter", wood will take about 1 minute to ignite, while gasoline ignites near instantaneously.

  • Fires on terrains and furniture are far less destructive. Large fires in structures are unlikely to destroy the entire building, however damage will still be substantial.
@Night-Pryanik

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 10, 2018

See item::is_tinder() for more information on what does or doesn't count as tinder.

Most players won't look into the code for this. Is there any information that should be available for players ingame?

@Night-Pryanik Night-Pryanik added Game: Balance Fields / Furniture / Terrain / Traps labels May 10, 2018

@NotFuji

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 10, 2018 •
edited

They get an entry in their description saying it will work as tinder.
paper
If it's not usable, no entry shows.

To be specific, tinder needs to be: fuel/4 < vol in L < fuel/2
For items that stack like ammo, it's: vol in L per charge < fuel/2, and charges > stack size/2
(stack size being the number of charges that make 250mL in most cases)

Wood items it comes out to anything between 125mL and 250mL
But it's different for every material

@Wishbringer Wishbringer referenced this pull request May 14, 2018

Merged

Zombie burner #23729

@Leland

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 15, 2018

For posterity: the original PR is #23677
@Leland

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 16, 2018

Haven't tested to see if this was pre-existing:

If you shoot a flamethrower at a building wall, it seems to instantly destroy it and dump debris everywhere

screen recording 2018-05-15 at 08 08 pm
@NotFuji
Tweak 
 - Logic fix
 - Lowered chance for fire to destroy tiles
Loading status checks…
26b5eef
@NotFuji

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 16, 2018 •
edited

Lowered the chance for smaller fires to destroy terrains, so while it might still happen, there's only a 0.7 - 1% chance per tick of it happening, rather than the 2 - 100% it was before.

But this also means buildings are harder to fully inflame with single flamethrower shots without any items to burn. Full auto consistently starts large fires.
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/field.cpp Outdated
if( cur.getFieldDensity() > 1 &&
one_in( 200 - cur.getFieldDensity() * 50 ) ) {
if( cur.getFieldDensity() >= FIRE_SIZE_THRESHOLD &&
one_in( 250 - ( cur.getFieldDensity() ) * 50 ) ) {

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018

Contributor

Why the () around cur.getFieldDensity()?
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/item.cpp Outdated
fuel += bd.fuel;
}

float vol = base_volume().value() / 1000.0f;

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018 •
edited

Contributor

Please use something like to_milliliter( base_volume() ) so a reader has a better understanding what range to expect here. And maybe change the name of vol to vol_in_liter.

Or just use to_liter (it returns a double) and remove the divison with 1000.
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/iuse_actor.cpp Outdated
inventory &inv = g->u.inv;
item &tinder = inv.find_item( inv.position_by_type( "tinder" ) );
if( has_tinder_here ) {
*density = 2;

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018

Contributor

As you seem to use density without checking it for being null, you pretty much require callers to supply a valid pointer as argument. Why is it a pointer and not a reference?
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/iuse_actor.cpp Outdated
return true;

inventory &inv = g->u.inv;
item &tinder = inv.find_item( inv.position_by_type( "tinder" ) );

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018

Contributor

As you're using tinder only inside one of the branches below, it should only be created there (as close to its usage as possible).
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/iuse_actor.cpp Outdated
inv.remove_item( &tinder );
}

p.add_msg_if_player( m_info, string_format( ( "You use %d charges of tinder to light the fire." ),

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018 •
edited

Contributor

Why are there () around the string literal. And it should be translated. Maybe only the _ is missing? Same below.

Also: you don't need to call string_format here, add_msg_if_player does string formatting on its own: add_msg( "You use %d", charges ).
@BevapDin

BevapDin reviewed May 19, 2018
View changes

src/fire.h
@@ -2,6 +2,9 @@
#ifndef FIRE_H
#define FIRE_H

const int MAX_FIRE_SIZE = 4;

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 19, 2018

Contributor

Have you tested this carefully? We currently only support field density up to 3 (MAX_FIELD_DENSITY is 3) and the fields data (inside of field_t) has only data for that many density levels.

A field density of more than 3 will lead to errors or just undefined behaviour.

This comment has been minimized.

Sign in to view
Copy link
@NotFuji

NotFuji May 20, 2018

Author Contributor

Resolved: see commit 1d72fd0

NotFuji added some commits May 20, 2018

@NotFuji
Requested Changes 
 - Max field density defined in field_t, appropriate code changed to use it.  Shouldn't cause problems anymore. (Hopefully)
 - Adjusted fire damage to use arbitrary max density
 - Cleaned up is_tinder()
 - Changed prep_firestarter_use to return an int density instead of modifying density pointer
Loading status checks…
1d72fd0
@NotFuji
fix
Loading status checks…
5217efd
@NotFuji
Comment
Loading status checks…
68617e4
@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jun 30, 2018

Can you resolve conflicts, please?
@NotFuji

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Jul 1, 2018

I'll probably end up redoing this in a way that's more compatible with the current field's system (and more reliable overall). And split the lighter rebalance into a separate PR. It works as-is, but I'm not quite sure if it's fun or balanced at all, nor do I have the time currently to find out.

@ZhilkinSerg ZhilkinSerg added the (S5 - OnHold / Stalled) label Jul 1, 2018

@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jul 1, 2018

I'll probably end up redoing this in a way that's more compatible with the current field's system (and more reliable overall). And split the lighter rebalance into a separate PR. It works as-is, but I'm not quite sure if it's fun or balanced at all, nor do I have the time currently to find out.

Okay. I will mark it as Stalled and close to clean up PR queue, but feel free to reopen it.

@ZhilkinSerg ZhilkinSerg closed this Jul 1, 2018

@Vasyan2006 Vasyan2006 referenced this pull request Feb 5, 2019

Closed

Lighting a fire is hindered by wind and rain #28065

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.