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

refactor: Energy Rework #5259

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

KheirFerrum
Copy link
Collaborator

@KheirFerrum KheirFerrum commented Aug 27, 2024

Checklist

Required

Optional

  • This is a C++ PR that modifies JSON loading or behavior.
    • I have documented the changes in the appropriate location in the doc/ folder.
    • If documentation for this feature does not exist, please write it or at least note its lack in PR description.
    • New localizable fields need to be added to the lang/bn_extract_json_strings.sh script if it does not support them yet.
    • If applicable, add checks on game load that would validate the loaded data.
    • If it modifies format of save files, please add migration from the old format.

Purpose of change

Basically, make batteries use discrete charges in the form of units::energy instead of charges, which will allow us to make power_draw behaviour less cursed and in general make code that handles power more reliable.

Describe the solution

There's a lot to write down here, I'm mostly past the early hurdles, and will now put down checkboxes for myself to tick off as I go to make sure I don't forget things.

  • Convert all batteries to battery type items. Make battery type items accept units::energy charges, convert most tools to do the same.
    • Make it so that max energy returns 0_kJ if the item is not meant to contain power, avoid the situation with charges where items can gain charges to infinity.
    • Write code for battery adaptors.
    • Figure out if I want this to be a consideration in magazine_integral(), I don't think so, but it's important I give this thought once the project is fully structured.
  • Convert iuse and iuse_actor to return the appropriate charge and power pair so the game knows how much of either to use from an item.
    • Update all the activities to use charges/power as appropriate.
  • Update recipes to appropriately use energy from tools.
  • Check that the grid functions correctly.

Describe alternatives you've considered

  • Keeping our shitty power_draw code
    • Tempting, and if I am not able to complete this within the next 2 weeks, likely.

Testing

Additional context

Procrastinated long enough. This commit is mainly proof of concept, it makes batteries type: BATTERY instead of type magazine and has them store energy as units::energy instead of charges.

`ammo_consume` is a hack for me to play with, which confirms that power is being drawn correctly when it is called.

The bionic scanner is sort of necessary at the moment for testing purposes.

Next step is to convert energy using functions over from charge usage.
@github-actions github-actions bot added src changes related to source code. tests changes related to tests data labels Aug 27, 2024
@@ -576,7 +576,6 @@
"target": "bionic_scanner_on",
"msg": "You start scanning for bionics.",
"active": true,
"need_charges": 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this will if I recall let you turn it on even when it has zero power? Unless 1 is a default in the code already.

Copy link
Collaborator Author

@KheirFerrum KheirFerrum Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Necessary while I'm testing, since batteries don't have charges in the rework. There's not a whole lot of point in reviewing my work at this stage, it's very preliminary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, aight. Hope it goes well then. :D

@scarf005 scarf005 added JSON related to game datas in JSON format. and removed data labels Aug 28, 2024
Add and convert most of the power handling code.

Need to update `has_energy()` and `use_energy()` pending an understanding of `visitable.cpp` code.
Finishes all the power draw code, so items that consume power via power draw will now consume power properly.

Next up is the ability to load batteries, unload batteries, and use magazines alongside batteries.
Copy link
Contributor

autofix-ci bot commented Aug 29, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

KheirFerrum and others added 2 commits August 30, 2024 00:43
Allow you to reload batteries into an item, add migration of batteries to new system. Lay down early framework for gun power draw system.

Energy Rework Part 3

Allow you to reload batteries into an item, add migration of batteries to new system. Lay down early framework for gun power draw system.
@KheirFerrum KheirFerrum force-pushed the The-BIG-FUCKING-energy-rework branch from 45e1a1b to b70c8f2 Compare August 29, 2024 23:44
autofix-ci bot and others added 5 commits August 29, 2024 23:45
set and mod energy should be able to modify non-battery items
`iuse_actor.cpp` use functions now return charges to be used and energy to be used as appropriate.

Updates partially some of the jsons that use batteries as ammo
@KheirFerrum KheirFerrum force-pushed the The-BIG-FUCKING-energy-rework branch from 13a13e6 to 7ecabae Compare August 31, 2024 20:42
autofix-ci bot and others added 6 commits October 2, 2024 19:56
Partially updates the iuse.cpp action. Fixes up the things the merge fucks in the code.
Continue work on iuse.cpp

Note, check up on pet food, it used to consume charges from via code instead of the return function. Must confirm if that will still work.
Starting with some macros, it's working alright. Updated the firework iuse actions to use the new transforms, removed the `taser2` iuse.

Remind myself to update the hand crank to actually recharge batteries after I'm done here.
@KheirFerrum KheirFerrum force-pushed the The-BIG-FUCKING-energy-rework branch from 098444c to bafda51 Compare November 21, 2024 21:29
No need to bother calculating energy/units remaining if the value to compare against is 0, should save on processing time.
Finally finished editing iuse.cpp.

Almost certain that I've missed something through all these.
Removes obsoleted iuse actors, updates npctalk to make use of new iuse return format. Update a test I missed previously. Updates JSONs
@KheirFerrum KheirFerrum force-pushed the The-BIG-FUCKING-energy-rework branch from 83fd1db to 23c6368 Compare December 4, 2024 15:16
KheirFerrum and others added 4 commits December 4, 2024 23:21
Updates to obsolete items to bring in line after I removed `fireweapon_on` and `fireweapon_off`

Migrates some items to the new battery system. More to be done later.
Updates to process_tool code to make it use power from an item, update visitable code to work around some weirdness with `sum_no_wrap`
@github-actions github-actions bot added the mods PR changes related to mods. label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. mods PR changes related to mods. src changes related to source code. tests changes related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants