[Issue] Energy Detector sometimes voids energy, or is an infinit energy source without input
bessw opened this issue ยท 35 comments
Describe
Expected
- measures the throughput and allows to limit it
- input and output of the Energy Detector are always the same
- no energy is lost (or only a bit as energy consumption of the Energy Detector)
Observed
- one time it suddenly was an infinity energy source even without an input attached (it gave 1.6 kFE/t)
- now it drains any available energy on the input and voids the excess
- multiple in parallel share the input evenly
- limiting the throughput works, but it is used up to max even without an attached cable
- two of our Energy Detectors are limited to 0 and don't have an input source attached, but still report a throughput of last time it was transmitting power
Reproduce
Steps to reproduce the behavior:
- use an not infinite energy source (Bigreactors - Basic Reactor)
used mods
- Bigreactors - Basic Reactor
- Flux Networks
- Rftoolspower - Powercell (High)
- Makanism - Basic Universal Cable
- many others not directly interfacing with this (ask if you need to know more)
Versions:
- Forge version: 36.1.4
- AdvancedPeripherals version: 0.4.4b
FYI, Rebooting the PC did not fix the report problem of the third observed behaviour.
By looking at EnergyDetectorTileEntity.java
it looks to me like the internal input and output of the Energy Detector is in fact a two times int_max sized energy storage, thus what we thought is voided is instead stored in the Energy Detector and is later what we thought is an infinity energy source.
When you use setTransferRateLimit(EnergyDetectorPeripheral.java) these values will override. But yes, that is the issue. I will try to fix that today if I can find time.
Hey, could you try if this version fixes your issue?
Unzip the file.
advancedperipherals-0.4.5-DEVb.zip
No, its still the same.
I also tested it in a new generated world:
- the one in the front still pulls energy at max transferrate of the cable
- for the one in the back:
- I fist hooked the Energy Detector up like the one in the front and let it charge up (without an output connected)
- now it is outputting the previously stored energy at max transferrate of the cable
- energyDetector.getTransferRate() always returns the last input rate > 0 even if it isn't inputting/outputting any energy
advancedperipherals-0.4.5-DEV1b.zip
I think, I fixed it. In any case, I can no longer reproduce it.
Unzip the file before you use it.
Ahh yes, I can reproduce this from you above too. The energy detector eats the energy because the storage is quite infinite big.
I fixed that.
advancedperipherals-0.4.5-DEV2b.zip
There is now also a config option for the maximum flow range.
You can test it if you want.
Unzip the file before you use it.
While im testing just a thought:
I don't know the Energy Transfer Api, but is there a specific reason why you need an internal storage on the Energy Detector?
Is it maybe possible to directly transfer the energy from the source block to the output block?
Ahh yes, I can reproduce this from you above too. The energy detector eats the energy because the storage is quite infinite big.
I fixed that.
advancedperipherals-0.4.5-DEV2b.zip
There is now also a config option for the maximum flow range.
You can test it if you want.
Unzip the file before you use it.
Now it doesn't transfer any energy. (I also tried it in a new world)
I changed the system of the energy detector completely. It will send the energy from the tile entity of the one side to the tile entity of the output side. It will no longer store any energy.
I will release it tomorrow
Ok great now its transferring energy and it doesn't store energy, but sadly there are new problems:
The Energy Detector is now actively draining energy out of the cable (instead of passive receiving it), this has the consequence that if there isn't enough energy for all devices that need energy the Energy Detector takes as much as it needs and all other devices only get the rest (if there is any).
This behavior is different to how cables normally work, because normally they would evenly distribute the available energy over the devices.
idea for solution:
You could implement a proxy class for the energy storage on the output side and set it as the "internal storage" of the Energy Detector:
class EnergyStorageProxy implements IEnergyStorage
...- when ever the cable tries to put energy into the Energy Detector the EnergyStorageProxy looks if there is an output device and forwards the the method calls to it
- if there is no output device
getMaxEnergyStored()
= 0
maybe also canReceive() -> false but this might break other stuff if it only checks once if canReceive() is false and then ignores this block ๐ค EnergyStorageProxy.receiveEnergy(inEnergy, ...)
should be something likeEnergyStorage out = getOutputDevice() if (out == null) { return 0 } int transferred= out.receiveEnergy(Math.min(inEnergy, maxTransferRate), ...) setTransferrate(transferred) return transferred
Btw. the current transferrate returned by the peripheral shows sometimes old data.
Maybe you should sum transferred
over one tick and check on every tick if you need to update the peripheral, because receiveEnergy() could be called multiple times.
I fixed that, that also fixed that the peripheral shows old data. ๐๐ป
Great, I will take a look at it once the update is out.
As far as I understand your code in your latest commit 12f1c52 you just moved the "if no input or no output return null" to the top of the method, or did I miss something?
How does this fix that the energy detector is actively draining energy from the input?
I already set the transfer rate to the received energy.
I just wrote setTransferrate(transferred)
because how you update the transferrate is not relevant for what I wanted to suggest, maybe that was more confusing then helpful.
Actually I was thinking about how the receiveEnergy(int maxReceive, boolean simulate)
Method of a EnergyStorageProxy
class could look like to handle passively receiving energy, this EnergyStorageProxy
would then be set as the internal storage of the energy detector.
Mekanism can't send the energy if the energy detector use it.
If you decrease the max transfer rate of the second energy detector, the universal cable will be able to send energy to the other cable.
My point was that it behaves differently if I replace the second energy detector with a universal cable and that my expectation was that a energy detector would behave the same like a universal cable as long as both don't exceed their max transfer rate.
(at least in this setup, I know that the energy detector behaves as a diode, but shouldn't make a difference in this setup)
you just moved the "if no input or no output return null" to the top of the method, or did I miss something?
I used && instead of || before, so the energy detector will drain energy even if there is no tile entity.
I just wrote setTransferrate(transferred) because how you update the transferrate is not relevant for what I wanted to suggest, maybe that was more confusing then helpful.
I now know what you mean, sorry. I'll look into it tomorrow.
My point was that it behaves differently if I replace the second energy detector with a universal cable and that my expectation was that a energy detector would behave the same like a universal cable as long as both don't exceed their max transfer rate.
Sorry, but I do not know how I could fix that. I'll look into it tomorrow, but I can't guarantee anything
The Energy Detector is now actively draining energy out of the cable (instead of passive receiving it)
I fixed that, that also fixed that the peripheral shows old data. ๐๐ป
EnergyStorageProxy.receiveEnergy(inEnergy, ...) should be something like
I already set the transfer rate to the received energy.
This behavior is different to how cables normally work, because normally they would evenly distribute the available energy over the devices.
Mekanism can't send the energy if the energy detector use it.
If you decrease the max transfer rate of the second energy detector, the universal cable will be able to send energy to the other cable.
I think I fixed the transferring so that it works as I expected,
but now I'm still searching the reason why the current transferrate returned by the peripheral shows sometimes old data.
I'll look later into it again and create a pull request after some more experiments.
I'll look later into it again and create a pull request after some more experiments.
Thank you for your help ๐๐ป
why the current transferrate returned by the peripheral shows sometimes old data.
I can't reproduce that, the detector returns always the perfect data
I should review before I merge a pull request...
The energy detector do not send energy out/do not receive energy.
I hadn't much time to look into this, sorry if I misunderstand anything.
But could you try to fix that?
That's strange...
I just checked out your master branch after the merge and tested it locally, for me it works without any problems.
Could you please tell me what you did / how you hooked it up, so I can try to reproduce it?
It does send energy, but it still seams like there is strange stuff happening with universal cables.
using the Network Reader at the input cable:
using the Network Reader at the output cable:
-
The input cable seems to call
receiveEnergy(...)
4 times per tick => yes my code still makes it possible that the maxTransferrate can be bypassed by using a cable that callsreceiveEnergy(...)
multiple times per tick. The only difference to your code is that mine prints the sum of the transferred energy of all calls toreceiveEnergy(...)
in the last tick and yours only prints the transferred energy from the last call toreceiveEnergy(...)
. -
What also confuses me is that the cable after the energy detector has a throughput of 9.5FE/t but the induction Matrix only receives 5FE/t like the actual limit and the input cable was.
Could it be something because getCapability(...)
returns for both sides the same storage?
There is an issue with your commit.
You need to use Optional#isPresent
instead of isEmpty
. isPresent
would return true if the Optional isn't empty. So you should use !isEmpty
.
The issue is at EnergyDetectorTileEntity.java:103. I could fix that, but i'm busy. Sorry
The if block doesn't need to be entered if outReceivingStorage is present, because it is only there to refetch the capability if the lazyOptional has been invalidated.
This is just a performance optimization because the documentation says:
Modders are encouraged to cache this value, using the listener capabilities of the Optional to be notified if the requested capability get lost.
Yeah, I need to use java 8, sorry.
Well, I used isPresent
without the !, so now I know why the energy detector does not work for me.