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

Energy density for batteries #75361

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented Aug 1, 2024

Summary

None

Purpose of change

Fixes #75336
Vehicle batteries seem to be fine, but a lot of portable batteries were egregiously to energy dense.

Describe the solution

Added an energy density test that asserts energy and specific densities for batteries based on battery chemistry and fixed the outliers (as you can see a lot of them were waaaay off).
This excludes test and debug batteries, and batteries from mods other than dda.

Describe alternatives you've considered

LiON in particular is tricky, there is hypothetically some room in there for more rare/premium battery cells to be found somewhere, so that door is open, but I don't have any intention of adding it. FYI this is like a 15% extra bump to capacity, not something crazy.
I checked adding tools to this as well, but it turns out they were all passing anyway and once I pivoted to making different assertions about different battery types it would have required a huge list mapping tools to battery type, so no thanks.

Testing

Tests continue to pass.
As commented after I first posted this, some items might have issues with their largest loadable battery being too low capacity to operate them, I'll see what I can do about that, but it's possible some items will be squeezed out of existence since they only ever worked because our battery capacities were broken.

Additional Context

This broke sticking a UPS in a welder and using it to repair your vehicle, as the heavy plus battery a UPS defaults to no longer has 1000kJ of capacity. This was a ridiculous and practically broken thing to do in the first place so that's fine.
We might want to follow up and remove the UPS-mod from the welder and actually turn it to a real boy welding cart instead of the weird gamey hand tool it's been since OG Cataclysm, but I don't want to scope creep this PR.
If you have a fully charged car battery you can use that to run a welder to do 10x repairs, that's a still kinda silly option but better than using portable tool power packs for it, which is pretty much what the heavy battery packs represent.

The power requirement for the gunsmith kit is kind of weird in the first place, if your gun is damaged you aren't soldering it back into pristine shape, ideally guns would get faults like having their sights misaligned or furniture broken, which would be more flexibly fixable, but on the other hand representing what happens to your gun when you're in melee is a pretty low priority.
Likewise if we're talking damage from firing, that should also be gun faults, and likewise fixable with repairing or replacing gun components, not waving a soldering iron over it and sacrificing some power and solder to the gun gods.

As for the power armor, use an atomic battery, they have significantly more capacity, that's the whole point of them.

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Items: Battery / UPS Electric power management Spawn Creatures, items, vehicles, locations appearing on map Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Aug 1, 2024
@kevingranade kevingranade force-pushed the energy-density-for-batteries branch 2 times, most recently from 844b669 to ecf84e9 Compare August 1, 2024 03:47
@SirPendrak
Copy link
Contributor

SirPendrak commented Aug 1, 2024

If i remember correctly, smartphone (120) dissasembles into one light battery (now 50cap.). Bionic storages mk I and mkII (100cap. and 250) dissasemble into 1 light battery (50) for mkI and 2 light, 1 u. light (120) for mkII.

Disposable batteries have line saying "The battery's chemistry means that it has a very high capacity, but cannot be recharged." But now they arent better than rechargeable ones, this might make players confused.

Power armors use heavy batteries. Now they will last even shorter time than before. They probably should accept car batteries/acumulators instead?

Players can craft bigger batteries using smaller ones. 2 u.l.b. -> 1 l.b; 3 u.l.b. -> 1 l.b.h.c. It would be better if u.l.b. had exactly 1/2 capacity of l.b., to avoid magical gains in capacity after crafting

@John-Candlebury
Copy link
Member

John-Candlebury commented Aug 1, 2024

How's this test run?

I'm mostly wondering why the aftershock "energy_cartidges" arent tripping it up.

@Bone-blood-moss
Copy link

Bone-blood-moss commented Aug 1, 2024

Why not add new batteries Increase the weight of the old batteries. Cos am i 99% sure, are they gonna be Readded and need we more batteries

@RanikOrtega
Copy link
Contributor

Looks like good changes. I just wonder if energy consumption in devices will now be off. AKA will a flashlight last as long as it should in real life etc etc. This may require a lot of changes to energy consumption in devices.

@Uwuewsky
Copy link
Contributor

Uwuewsky commented Aug 1, 2024

So ultra-light disposable battery have same stats as non-disposable one?

And I just checked, a firearm repair kit still requires batteries to be inserted, and some operations like adding side mounts require exactly 100 charges. Which will be impossible unless you add a medium battery mod to the kit.

@RanikOrtega
Copy link
Contributor

So ultra-light disposable battery have same stats as non-disposable one?

And I just checked, a firearm repair kit still requires batteries to be inserted, and some operations like adding side mounts require exactly 100 charges. Which will be impossible unless you add a medium battery mod to the kit.

Good points. That's what I was worrying about. This change is not a small one since a lot of other items depend on batteries.

@SariusSkelrets
Copy link
Contributor

How does it affects modded batteries, such as XE's inventor batteries?

I looked at the test and saw nothing in it that accounts for mods' sci-fi/anomalous batteries

@kevingranade
Copy link
Member Author

If i remember correctly, smartphone (120) dissasembles into one light battery (now 50cap.). Bionic storages mk I and mkII (100cap. and 250) dissasemble into 1 light battery (50) for mkI and 2 light, 1 u. light (120) for mkII.

Sure I can bump them down too.

Disposable batteries have line saying "The battery's chemistry means that it has a very high capacity, but cannot be recharged." But now they arent better than rechargeable ones, this might make players confused.

I can drop them more sure.

Power armors use heavy batteries. Now they will last even shorter time than before. They probably should accept car batteries/acumulators instead?

Weird it's almost like conventional batteries are insufficient for running power armor, these were supposed to be using supertech batteries, what hapened?

Players can craft bigger batteries using smaller ones. 2 u.l.b. -> 1 l.b; 3 u.l.b. -> 1 l.b.h.c. It would be better if u.l.b. had exactly 1/2 capacity of l.b., to avoid magical gains in capacity after crafting

Yea that's valid, will do.

I'm mostly wondering why the aftershock "energy_cartidges" arent tripping it up.
How does it affects modded batteries, such as XE's inventor batteries?

We don't run tests against any mods except Magiclysm AFAIK, to future proof this I could change the checks that exclude batteries defined in the test data to excluding anything that isn't just defined in the dda mod, and if mods want to use the test we could switch that up to have per-mod thresholds.

Why not add new batteries Increase the weight of the old batteries. Cos am i 99% sure, are they gonna be Readded and need we more batteries

I really didn't want to put all the change into weight/volume because that is even more likely to make batteries not fit in things they currently fit in, which is a much worse failure than just losing some capacity.

Looks like good changes. I just wonder if energy consumption in devices will now be off.

They were supposed to be scaled to wattage of the device it's based on, if a device had its power consumption erroneously set based on run time with inflated battery capacity, yes it will now skew low.

and some operations like adding side mounts require exactly 100 charges

What is this, soldering/brazing? If that's supposed to be a soldering iron I have bad news for you, it shouldn't work anyway. brazing a sight onto a gun needs a torch, not a soldering iron. As a stopgap yea we could just cram a bigger battery in there, but at the end of the day it shouldn't be doing that in the first place.

Thanks for the feedback, I'll see about what addressing these looks like.

data/json/items/battery.json Outdated Show resolved Hide resolved
@x-qq

This comment was marked as off-topic.

@kevingranade kevingranade force-pushed the energy-density-for-batteries branch from ecf84e9 to 9dabc8c Compare August 2, 2024 21:05
@kevingranade
Copy link
Member Author

Which will be impossible unless you add a medium battery mod to the kit.

The gunsmith repair kit already accepts medium batteries???

@kevingranade kevingranade force-pushed the energy-density-for-batteries branch from 9dabc8c to 86570af Compare August 2, 2024 22:26
@github-actions github-actions bot added Vehicles Vehicles, parts, mechanics & interactions Crafting / Construction / Recipes Includes: Uncrafting / Disassembling EOC: Effects On Condition Anything concerning Effects On Condition labels Aug 2, 2024
@kevingranade kevingranade force-pushed the energy-density-for-batteries branch from 86570af to 86415a2 Compare August 2, 2024 22:52
@Uwuewsky
Copy link
Contributor

Uwuewsky commented Aug 3, 2024

The gunsmith repair kit already accepts medium batteries???

This one yes, but there are two different ones. A second one is small_repairkit-firearm repair kit is a little less advanced and uses light batteries.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 3, 2024
@Alm999
Copy link
Contributor

Alm999 commented Aug 3, 2024

Excuse me for interrupting, but what about power consumption of mundane items like mp3 players, flashlights or reading lamps?
As far as I know, even activating an mp3 player uses up 1 charge of power instantly so with ultra-light battery installed the battery loses half of its charge by simply flipping the switch. And an mp3 player shall last for at least several hours of usage on full charge.

IMO, the power consumption shall also be adjusted (with the "activation fee" of 1 charge being removed altogether) accordingly.

UPDATE

Upon further investigation and some research on mp3 players I have calculated that an USB stick mp3 player with swappable AAA battery (see http://www.agptek.com/index.php/product/agptek-u3-8gb-mp3-player-music-player-with-usb-flash-drive-recording-fm-radio-supports-up-to-32gb-black/ ) and proclaimed life time of "up to 10 hours" will give us approximately 2 mW of energy consumption (45 kJ / 36000 s = 0.00125 W or 2 mW because I do not entirely trust advertisement claims). However even if the game accepts the "2 mW" as a valid value for "power_draw" field in the JSON, it fails to function properly and active mp3 player with this value will function seemingly indefinitely (even after 6 hours of in-game waiting the charge did not drop at all). So, maybe something in the game code needs to be reworked so appliances could have sub-watt consumption.

@dynst
Copy link
Contributor

dynst commented Aug 3, 2024

(45 kJ / 36000 s = 0.00125 W or 2 mW because I do not entirely trust advertisement claims).

Isn't the capacity of a single rechargeable AAA battery more like 3.7V 350mAh (4.6 kJ), not 45 kJ? So the power draw wattage would be about 1/10th that: 0.2 mW.

@Alm999
Copy link
Contributor

Alm999 commented Aug 3, 2024

Isn't the capacity of a single rechargeable AAA battery more like 3.7V 350mAh (4.6 kJ), not 45 kJ? So the power draw wattage would be about 1/10th that: 0.2 mW.

I was using the "light_disposable_cell" stats proposed by this PR (1 charge = 1kJ). I just assume that an ultra-light cell is something like CR2032 tablet cell and a light variant is something like AAA/AA hybrid.

@kevingranade
Copy link
Member Author

So, maybe something in the game code needs to be reworked so appliances could have sub-watt consumption.

Yes this is a longstanding preexisting issue. We are trying to handle power amounts ranging from individual mJ up to many GJ (the contents of an entire power grid with many large storage batteries) and that leads to difficulties in one end it the other. Other than rendering light minus cells essentially useless (except they already were?) this PR doesn't change the fundamental problem.

@Maleclypse Maleclypse merged commit 2c0f4c1 into master Aug 6, 2024
22 of 27 checks passed
@Maleclypse Maleclypse deleted the energy-density-for-batteries branch August 6, 2024 05:30
@GuardianDll
Copy link
Member

There are migration issues i suspect - pure electrons drop on the ground
image-66

@Istoprocent

This comment was marked as off-topic.

@GuardianDll
Copy link
Member

GuardianDll commented Aug 7, 2024

If someone has problem about this change, please make a separate issue

@CleverRaven CleverRaven locked as too heated and limited conversation to collaborators Aug 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. Crafting / Construction / Recipes Includes: Uncrafting / Disassembling EOC: Effects On Condition Anything concerning Effects On Condition Items: Battery / UPS Electric power management [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Spawn Creatures, items, vehicles, locations appearing on map Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

batteries are more energy dense than plutonium