Advanced Peripherals

Advanced Peripherals

29M Downloads

[Issue] Energy Detector sometimes voids energy, or is an infinit energy source without input

bessw opened this issue ยท 35 comments

commented

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

  1. one time it suddenly was an infinity energy source even without an input attached (it gave 1.6 kFE/t)
  2. 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
  3. 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:

  1. 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
commented

FYI, Rebooting the PC did not fix the report problem of the third observed behaviour.

commented

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.

commented

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.

commented

Oops, I didn't mean to do that .

commented

Hey, could you try if this version fixes your issue?
Unzip the file.
advancedperipherals-0.4.5-DEVb.zip

commented

No, its still the same.

I also tested it in a new generated world:
image

  • 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
commented

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.

commented

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.

commented

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?

commented

Well, this is a very good idea.
I will try that out when i am at home again.

commented

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)

commented

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

commented

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.

image

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 like
    EnergyStorage 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.

commented

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)

commented

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

commented

I just managed to build your mod on my pc, I will also try to take a look into it.

commented

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.

commented

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.

commented

I'll look later into it again and create a pull request after some more experiments.

Thank you for your help ๐Ÿ‘๐Ÿป

commented

why the current transferrate returned by the peripheral shows sometimes old data.

I can't reproduce that, the detector returns always the perfect data

commented

I will reopen this issue until the 0.4.6b update is released.

commented

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?

commented

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?

commented

image

image

It does not seem that you send the energy? Or do I misunderstand anything?

commented

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:
input cable

computer

using the Network Reader at the output cable:
output cable

storage

  • 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 calls receiveEnergy(...) multiple times per tick. The only difference to your code is that mine prints the sum of the transferred energy of all calls to receiveEnergy(...) in the last tick and yours only prints the transferred energy from the last call to receiveEnergy(...).

  • 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?

commented

But I still don't see any reason why yours doesn't transfer energy...

commented

I will try to merge the pull request and see if that works for me.

commented

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

commented

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.

commented

I mean, isEmpty does not even exist.
image

commented

image

oops that's a java 11 feature, sorry

commented

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.

commented

Stupid auto close... ๐Ÿ˜„

commented

image

Well, thank you for your help! I really appreciate that!

commented

Fixed in 0.4.6b