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

Remove unnecessary energy cost for turning devices on/off #75903

Closed
wants to merge 2 commits into from

Conversation

taatu
Copy link
Contributor

@taatu taatu commented Aug 23, 2024

Summary

Balance "Remove unnecessary energy cost for turning devices on/off"

Purpose of change

With battery capacities being reduced, the 1 kJ cost to turn on devices has become overly punishing.
Roughly 5-4 years ago item::process_tool was reworked to get rid of the "power consumption time threshold". the power draw of an item is randomly timed, so there are no more rounding errors that can be exploited.

Describe the solution

remove the charges_per_use from turning on and off electronic items that have a continuous power draw (eg. "power_draw": "1 W". Items still using the deprecated turns_per_charge are excluded, except when they have both a separate turn-on and turn-off cost)

Describe alternatives you've considered

Testing

Additional context

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Items: Armor / Clothing Armor and clothing Game: Balance Balancing of (existing) in-game features. json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Aug 23, 2024
@x-qq
Copy link

x-qq commented Aug 23, 2024

fixes #75620
fixes #75658
fixes #75632

(in theory)

@Maleclypse
Copy link
Member

The desired fix for this will either be "electronic activation consumes the fraction of energy and doesn't round to kj" or, as Ren suggested, "if activation cost is less than 1, we roll remainder to consume energy"

@Maleclypse Maleclypse closed this Aug 23, 2024
@taatu
Copy link
Contributor Author

taatu commented Aug 23, 2024

I think you are misunderstanding the situation. The fix was implemented 5 years ago, all the activation costs this PR removes (except maybe the smartphone) are artifacts of the previous workaround, not based on reality. And the reason they were not removed already is because in most situations their effect was negligible.

@Maleclypse
Copy link
Member

@RenechCDDA @kevingranade I believe you two have a better understanding of this area of code than I do so please reopen this PR if I've misunderstood.

@taatu
Copy link
Contributor Author

taatu commented Aug 23, 2024

the relevant code block should be

} else if( type->tool->power_draw > 0_W ) {
    // kJ (battery unit) per second
    energy = units::to_kilowatt( type->tool->power_draw );
    // energy_bat remainder results in chance at additional charge/discharge
    const int kw_in_mw = units::to_milliwatt( 1_kW );
    energy += x_in_y( units::to_milliwatt( type->tool->power_draw ) % kw_in_mw, kw_in_mw ) ? 1 : 0;
}

where x_in_y is the random function. I've tested this in-game with a flashlight, and it seems to work as expected with individual charges being lost at random intervals (so eg. if you try to cheat by turning a device with 1 watt of power draw on and off every 5 seconds, you still over time lose on average the correct amount of charge)

@kevingranade
Copy link
Member

That handles the power consumption part of the problem, what it doesn't mitigate is the problem where a player flicks a flashlight on and off within one turn, and enemies don't get a chance to react to that. The better solution for THAT IMO is if you turn on a light source, it does something like putting down a light-emitting field that sticks around until the start of your next turn and illuminates you. That might have actually happened at some point, because this isn't a new idea, I'd need to check.

@Luca-spopo
Copy link
Contributor

Luca-spopo commented Oct 13, 2024

putting down a light-emitting field that sticks around until the start of your next turn and illuminates you. That might have actually happened at some point, because this isn't a new idea, I'd need to check.

In the code I do see something which appears to do that: An effect called haslight

src/lightmap.cpp

static const efftype_id effect_haslight( "haslight" );

// Stuff

void map::apply_character_light( Character &p )
{
    if( p.has_effect( effect_onfire ) ) {
        apply_light_source( p.pos(), 8 );
    } else if( p.has_effect( effect_haslight ) ) {
        apply_light_source( p.pos(), 4 );
    }

    const float held_luminance = p.active_light();
    if( held_luminance > LIGHT_AMBIENT_LOW ) {
        apply_light_source( p.pos(), held_luminance );
    }

    if( held_luminance >= 4 && held_luminance > ambient_light_at( p.pos() ) - 0.5f ) {
        p.add_effect( effect_haslight, 1_turns );
    }
}

So the effect haslight gets applied for one turn, if the character is holding a lit object that is somewhat brighter than the ambient light at the location.
If the player has this effect, they get an extra 4 light applied to their position (for the entirety of the next turn)

@Luca-spopo
Copy link
Contributor

Luca-spopo commented Oct 13, 2024

Since this PR fixes some issues, I would like if it is re-opened @taatu @Maleclypse @RenechCDDA @kevingranade

There is also some concerns I have regarding the PR itself:
The activation for items can have conditions, such as "need_charges": 1
Here, one charge is 1kJ, so it will not allow the item to be activated even if it has fractional energy in it (less than 1kJ)

I think this PR was raised before #75912 was merged so its natural that it may not have accounted for the ability to have energy tracked at amounts smaller than a single charge

but now that we can account for energy less than one battery charge - these requirements should be changed to needs_energy instead of needs_charges, and should borrow from the energy_per_second for how much energy is required at minimum for activation to be possible

Other than that - some tests are failing - the tests need to be modified. The test is checking that some charge is immediately consumed on item usage. Since we're removing that here intentionally, that test can be removed. It should be replaced by a different test that confirms that switching on and then switching off a flashlight in the same turn does in fact consume some energy

If switching on and switching off within the same turn is NOT consuming energy (I didn't test, @taatu please tell), then the basis of this PR might be wrong. In that case we should replace the charges_per_use with energy_per_use and use up energy instead of charges

I do not have the full context, I am talking based on my reading of the code changes / discussions in this PR and in #75912

@Luca-spopo
Copy link
Contributor

Luca-spopo commented Oct 13, 2024

Update: I have tested - and the current changes in this PR let me use a flashlight for free as long as I keep switching it on and switching it off within the same turn. Essentially, activating the flashlight is free now.

I think that's going to be exploitable, even for other similar items.

I think we need to replace charges_per_use with power_draw_to_use or something similar, and allow us to specify activation cost in mJ

@Maleclypse
Copy link
Member

Update: I have tested - and the current changes in this PR let me use a flashlight for free as long as I keep switching it on and switching it off within the same turn. Essentially, activating the flashlight is free now.

I think that's going to be exploitable, even for other similar items.

I think we need to replace charges_per_use with power_draw_to_use or something similar, and allow us to specify activation cost in mJ

Ok so per your testing this PR needs to stay closed and a new PR doing something different would be needed to fix the issue you surfaced above? I'd suggest doing this in a new issue so it doesn't get lost for most people.

@Luca-spopo
Copy link
Contributor

Luca-spopo commented Oct 13, 2024

An issue already exists: #75632 so the same issue can be used for tracking

This PR is not the correct solution for flashlights etc. I think. Or any other item that benefits the player even if they simply switch it on-then-off within a single turn.

But the PR may be an acceptable solution for some other items that constantly drain energy and do not benefit from the "switch it off in the same turn" exploit. For example emf_detector (I think?) or noise cancelling headgear - which need to stay active passively across multiple turns

Never really used the emf detector so Im only assuming it's a passive thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions Game: Balance Balancing of (existing) in-game features. Items: Armor / Clothing Armor and clothing [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants