Applied Energistics 2

Applied Energistics 2

156M Downloads

Removing energy cards from wireless terminals doesn't clamp their power

Mari023 opened this issue · 4 comments

commented

Describe the bug

removing an energy card from a wireless terminal keeps the current charge, even if that energy is greater than the it can normally store
2023-10-04_02 45 58

How to reproduce the bug

  1. get a charged wireless terminal
  2. install an energy card
  3. charge the terminal a bit more, so it is more than 50% full
  4. remove the energy card
  5. the terminal is now more than 100% full

Expected behavior

the terminal should void the excess energy

Additional details

discovered in an ae2wtlib dev env, with the normal wireless terminal (ae2wtlib doesn't touch it)

Which minecraft version are you using?

1.20

On which mod loaders does it happen?

Fabric

Crash log

https://gist.github.com/Mari023/3df26bfd51ad3a089278dd00cb0f0dd7

commented

Side note: I‘m far from any PC to verify my thoughts or test my theories.

Looking at https://github.com/AppliedEnergistics/Applied-Energistics-2/blob/master/src/main/java/appeng/items/tools/powered/powersink/AEBasePoweredItem.java#L167 there are two cases. Either the multiplier is reset to 1 (which is your problematic case I think) or set to a larger value. Only in then the current power clamping code in setAEMaxPower() is executed. My guess is that a similar code should be in resetAEMaxPower() and is simply missing.

Depending on if the two functions are called from anywhere else there could also be room for refactoring the current power clamping code into setAEMaxPowerMultiplier() to avoid code duplication.

commented

I don't think resetAEMaxPower() is necessary at all, setAEMaxPower() already removes the tag when it is set to (or below) the default value

commented

In theory all three functions could be merged into one if you look at only that specific file. The crucial point is probably if there are external callers to the more specific functions elsewhere. Since this is an abstract class and the functions are protected in scope there could theoretically be some class that inherits from this base class that calls these functions directly.

If these functions are considered API-relevant, e.g. because some other mod relies on them it could be problematic to merge them. Then you could maybe reimplement resetAEMaxPower() to just call setAEMaxPower(stack, powerCapacity.getAsDouble()); and avoid code duplication that way.

commented

Still need to test it before I make a pull request out of this.