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

Use item::energy to stash sub-kJ power quantities for more precise power usage #75912

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented Aug 23, 2024

Summary

Bugfixes "Allow tiny continuous power draw in electronic devices."

Purpose of change

TODO find related issues
We've had issues with low-draw devices that are supposed to work for a very long time for practically forever. There are workarounds such as tool::turns_per_charge, but that's gnarly and has weird side effects.
The root cause is that the lower bound of charge resolution has been "one battery charge", which we set at 1kJ because there's also a rather large number of charges we need to handle for e.g. electric cars, and the "number of charges" type is an int.

Describe the solution

I poked around and we already have a member of the item class that is type units::energy which is backed by a int64_t and has a resolution of one mJ. Due to an int64_t being staggeringly large, this really can handle whatever we feel like throwing at it, so why isn't the problem fixed?
The problem is legacy code that treats electrical capacity as "charges" and a whole ton of infrastructure, mostly in crafting code, and specifies power drain in "charges of battery", which again are 1kJ each.
After rooting around though, I realized we don't have to overhaul all the crafting code to fix this particular issue, because the "process_tool()" code responsible for e.g. spending power every turn for an active flashlight doesn't interact with any of that code.
So what I've done here is I updated item::process_tool() to call item::energy_consume() and item::energy_consume() to use item::energy to stash residual energy as needed. So if your flashlight has 10 "charges" of battery and you try to spend 20J, it spends one "charge", but stashes the remaining 980J of energy in item::energy, which can be used next turn instead of burning more charges.

Describe alternatives you've considered

There's a huge overhaul I eventually want to happen here where the itype_battery item that represents a distinct amount of charge just goes away and instead a battery item like a light_battery instead just uses it's item::energy member and islot::battery::max_charges member to handle how much charge it can hold. That requires overhauling a huge amount of crafting code though and I'm not up to that at the moment.

Testing

Added tests for an exemplar, specifically having a small LED flashlight run for the intended 10 hours on one set of batteries.
Before this change it would round up from 1.5J per turn to 1kJ per turn, so it would run for one hour instead of about 10.

Additional context

See e.g. #75903 for other aspects of the same problem.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions labels Aug 23, 2024
@kevingranade kevingranade force-pushed the mJ-power-consumption-for-tools branch from d288a71 to 5701f36 Compare August 23, 2024 23:43
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Aug 23, 2024
@Hydrogen-Deuteride
Copy link

I think this "stash" thing should be on battery side. So then it can be also used to limit how much energy battery can output in 1 turn(essentially max power of the battery), so we dont have this situations when 10g battery released 2kJ of energy in 1 turn which is like 2kW of power provided.
Also player should see how much sub charges remains.

Unless this is just temporarly hack untill battery system overhaul.

@kevingranade
Copy link
Member Author

It is on the "battery" side, an example of a "battery magazine" is a lithium ion battery pack in a phone, or a AA battery cell.
The actual "battery" itype is a massless, volumeless unit of charge.

@kevingranade kevingranade force-pushed the mJ-power-consumption-for-tools branch from 06604a6 to 663ce38 Compare October 1, 2024 17:02
@github-actions github-actions bot added <Bugfix> This is a fix for a bug (or closes open issue) Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 1, 2024
@kevingranade kevingranade marked this pull request as ready for review October 1, 2024 19:39
src/item.h Outdated Show resolved Hide resolved
src/item.cpp Outdated Show resolved Hide resolved
src/item.cpp Outdated Show resolved Hide resolved
@Maleclypse Maleclypse merged commit f61d1cd into master Oct 3, 2024
26 checks passed
@Maleclypse Maleclypse deleted the mJ-power-consumption-for-tools branch October 3, 2024 04:51
@GuardianDll
Copy link
Member

Can't get how does it work json side? Does it just allow "charges_per_use: 0.5, or it allows string notation now?
Unless i misunderstood, and it doesn't actually cover electric tool activation?

@kevingranade
Copy link
Member Author

You could already specify "power_draw": "1560 mW", but it was rounding up every turn. I.e. the flashlight was set to that already, but it rounded up from 1560 mJ / second to 1 kJ per second. Now it actually consunes 1560 mJ / second and lasts about 10h as stated in the related product page.

Charges_per_use along with turns_per_charge works by evaluating that fraction, but the preferred solution is power_draw.

You are correct that this does not impact initial activation, that's all entangled with the iuse infrastructure and is still limited to charges.

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 BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants