IC2 Experimental causing Inconsistency with IC2 Classic regarding IMultiEnergySource
coderbot16 opened this issue · 14 comments
It appears that IC2E and IC2C interpret IEnergySource::getOfferedEnergy
differently when an energy tile is capable of outputting multiple packets through IMultiEnergySource
. In IC2E, getOfferedEnergy appears to return the amount of energy the tile is offering across all packets, but in IC2C getOfferedEnergy appears to return the amount of energy the tile is offering in each packet.
I found this issue while implementing multipacket support for TechReborn transformers. While they work fine in IC2E, in IC2C they blow up receiving machines. While it is possible for me to implement a workaround in TechReborn, I just wanted to let you know about the issue first.
On another note, it appears that the information on Curseforge regarding Team Reborn mods is out of date. Steve's Carts was actually fixed to not crash all the way back in August. In addition, I've also submitted many PRs to TR for IC2 Classic support and as of last Saturday the latest version of TechReborn supports IC2 Classic out of the box with no further mods or config changes needed.
@coderbot16 that you have to point to IC2Experimental since they have ported this from IC2Classic to IC2Experimental. (They don't apply the rules)
@Chocohead could you maybe take care of that and maybe fix the energynet of ic2exp to apply the rules of how the interface was designed.
Yeah but this is nothing i can really change since the Packet logic in IC2C handles multipackets basically as unique instances of calulation. So each packet gets its own calculation and is treated seperatly. On top of that this prevents an issue with IC2Exp where power gets so low if you can not provide the nessesary amount that it will basically waste a lot of eu.
Also thanks for the notice but this is IC2Exp side for not importing it properly.
My suggestion would be just have a global IC2Exp/Classic Flag and handle things based on the flag since its either: (isIC2Exp() 128 : 32)
And outside of ic2exp i can no longer report bugs because i did that so much with my kind that they say that they need other people to report the same bug before they even concider looking at them.
The documentation for the IEnergySource:getOfferedEnergy
function in this case says:
Maximum energy output provided by the source this tick, typically the stored energy. The value will be limited to the source tier's power multiplied with the packet count
Correct me if I'm wrong, but it appears that IC2 experimental behavior is in line with the documentation of the function. Instead of calling getOfferedEnergy on each iteration of the packet distribution loop, it seems like you should call getOfferedEnergy once and then decrement from it as needed within the loop. Basically if you have an LV transformer that offers 128EU of energy, then each iteration should take 32EU (LV tier rate) from the offering.
I assume the flaw you are referring to is if a transformer has 1 EU stored, it will be split up into 4 0.25 EU packets instead of a single 1 EU packet. That is a valid issue, however it is also possible to migitate it while still behaving like IC2 Experimental by not dividing up the EU evenly, and instead prioritizing creating full size packets.
At the very least, the E-net code should limit the output getOfferedEnergy based on the source tier like what the documentation says, since this will also prevent the machine explosion problems with multiple packets.
I can always implement a workaround as you say, but I just want to avoid special cases between classic and experimental.
@coderbot16 yes the normal IEnergySource version is fully right implemented, but what you talk about is IMultiEnergySource which means that packets are a thing.
This IMultiEnergySource interface was ported from IC2Classic To IC2Experimental and @Aroma1997 knew how the implementation was when he ported it to IC2Experimental.
Which means after @sfPlayer1 fuckt up the implementation of how I created it without thinking about IC2Classic.
This was the first implementation of IC2Classic did it in 1.7.10 before even experimental ported it over to its own System.
You can look that up in older ic2c versions it will look always the same there and you can find it in there in older IC2Exp versions the same way.
Here is a simple example of how the logic should work!
public void handleEnergy(IEnergySource source)
{
if(source instanceof IMultiEnergySource && ((IMultiEnergySource)source).hasMultiPackets())
{
IMultiEnergySource multi = (IMulitEnergySource)source;
for(int i = 0;i<multi.getPacketCount();i++)
{
int transfered = transferEnergy(source.getOfferedEnergy());
if(transfered <= 0)
{
return;
}
source.draw(transfered);
}
}
else
{
int transfer = transferEnergy(source.getOfferedEnergy());
if(transfer > 0)
{
source.draw(transfer);
}
}
}
@coderbot16 basically IC2Experimental has the IMultiEnergySource interface changed in this way:
public int handleEnergy(IEnergySource source)
{
int provided = source.getOffered();
int packetCount = source instanceof IMultiEnergySource ? ((IMultiEnergySource)source).getPacketCount() : 1; //skipping some validation code here.
for(int i = 0;i<packetCount && provided > 0;i++)
{
int transfered = transferEnergy(Math.min(provided, EnergyNet.instance.getPowerForTier(source.getSourceTier()));
if(transfered <= 0)
{
break;
}
provided -= transfered;
}
return provided;
}
Ofcourse a bit more optimized then this code, but this basically brakes the rules where "Each Packet gets treated as its own" because it creates 1 giant packet and then splits it up instead of asking the the source to handle that.
Merging my energynet from my system to their system basically breaks some of my Blocks because they rely on the fact that they can control each individual packet.
(1 Example would be the Battery Station allowing to send individual packets based on the batteries inside of it. So 1 packet can be 32EU a second one can be 512EU another one can be 8 EU etc etc)
That would be no longer possible with their EnergyNet implementation.
And since they copied from IC2Classic with my permission they have to fix it.
And just to ping @sfPlayer1 and @Aroma1997 again to make sure they read this.
Alright, so the issue goes deeper than I thought. Well, hopefully we can reach a resolution on the issue so that IC2E and IC2C can have fully compatible energy implementations.
One thing to consider is that depending on how many mods using the IC2E api use the multipacket API, they might not be able to change their implementation. Since if they did, it would have the exact same result as that which happens with TR right now under IC2C: multipacket emitters will cause things to explode. On the other hand, changing your implementation would at most cause machines to emit too little power, which while annoying wouldn’t have such catastrophic consequences.
@coderbot16 since 99% of IC2 Addons are developed by @Chocohead and he is a IC2Exp developer that should not be an issue, and other then that there should be no mod that has multipacket support in the first place.
On the other hand, as I said before, a way to fix this in your code without forcing anyone to change the semantics of their API is to just limit the return value of getOfferedEnergy by the tier of the machine, which would transparently prevent machine explosions as a
result of this issue.
(Just since it doesn’t appear that your code currently checks the return value against the machine tier like the IEnergySource API says)
yeah because for some machines that has to be disabled, note that this is only for multipacket machines where that is required... So i can not do that.
Also there is a couple machines that can exeed that. Example would be a nuclear reactor, since i only support tier system of 0-13 and not Integer.MAX_VALUE
That still does not justify the bulk packet logic split... My tool just has less sanity checks for things.
Anyway since we have here 2 unmoveable forces since the devs of exp don't care the solution stays to be done at your side since its the safest solutions for all sides, i can not fix it without breaking my own mod, they will not do it. So yeah make a flag and just change the Energy output lower per packet if classic is detected.
(Yes i pull the plug on this one because honestly i do already enough compat to IC2Experimental this is not going to be a thing)
@coderbot16 follow up. I talked with player directly and he basically wasnt aware that it was my Interface they have imported. Basically he said he sooner or later will sit down to rework the enet again, if he does not forget he will involve me in the API process and make something that is fine for Experimental & classic at the same time.
But this might take months before it even starts.