Crash with ic2 classic when crafting Industrial Grafter

[Meduris]

This happened on a heavily modded minecraft 1.10.2 server with gendustry 1.6.5.30 and ic2 classic version 1.4.0.0. I already asked the ic2 classic developer about this issue and he told me that this is a bug on your side for the following reason (quoted from him):

IC2 gets the Manager for the fitting item. If that returns null then there is no issue. but for the industrial grafter, it returns the one that is representing the "IElectricItem" one and the item doesn't implement IElectricItem. I wonder why that is not crashing in the first place when you use that item. Please implement the API Properly.

(crashlog follows as soon as github allows me to upload it)

[Meduris]

Link to the crashlog: https://pastebin.com/BL20fREj

[bdew]

It does work with normal ic2... I'd recommend the author of "classic" to not blame others for not implementing the api correctly if it works with the base mod :P

I'll look if i can somehow tweak it to work around the crash...

[Meduris]

I did not test it with ic2 experimental yet, but I made a custom version which fixed it for me (just using it in a private modpack).

What I changed was to remove the ItemPoweredEU interface from the ItemPowered interface and implement IElectricItem in IndustrialScoop and IndustrialGrafter. (Implementing IElectricItem fixed the tooltip crash, but with the ISpecialElectricItem implementation and your ItemPoweredEUManager, recharging in ic2 classic machines was not possible. Without the ItemPoweredEU interface charging those tools worked just fine).

If you'd like, I can look into fixing it for both mods this weekend. My current fix is just not that clean, as this was the first time for me to touch Scala and I had no need for it to work it ic2 experimental.

btw. I also had to add runDir = "run" to the minecraft{...} section of the build.gradle file to be able to set up the eclipse workspace

[bdew]

I've looked a bit and the difference between the 2 seems to be that in classic ElectricItemManager.getCharge calls discharge internally:

public double getCharge(ItemStack itemStack)   {
    return discharge(itemStack, 2.147483647E9D, Integer.MAX_VALUE, true, false, true);
}

While in experimental it routes it via ElectricItem.manager which then dispatches the call to my ItemPoweredEUManager:

public double getCharge(ItemStack stack)  {
   return ElectricItem.manager.discharge(stack, Double.POSITIVE_INFINITY, Integer.MAX_VALUE, true, false, true);
}

So when it calls getTooltip on my manager, i delegate the call to ElectricItemManager which in turn should do the underlaying getCharge/discharge calls via ElectricItem.manager (and does so in exp).

Moreover the API clearly says that the way i'm doing it is legit

• @note If you're implementing your own variant (ISpecialElectricItem), you can delegate to the
• default implementations through ElectricItem.rawManager. The default implementation is designed
• to minimize its dependency on its own constraints/structure and delegates most work back to the
• more atomic features in the gateway manager.

I don't want to go the IElectricItem because it's more pain in the ass to manage as a soft dependency (so it works even if IC2 api is not loaded). I could fix it by implementing those methods directly instead of delegating them to the default implementation... but i think it's something they should actually fix in classic.

[Speiger]

Basically what I did in the RawManager is removing back calls to the ElectricItemGateway manager because all those side checks create a lot of extra overhead. In other words in classic: When you call any function in the Rawmanager then ensure that you implement IElectricItem which ISpecialElectricItem no longer implements.

The reason why I did the constant gateway checks is: when you call the raw then it should be in every case IElectricItem.

So its my fault to remove sanity checks for people who make mistakes with the API. But its your mistake for using the API wrong.

The fix on your side would be to implement your own tooltip implementation... Litterly its: getCharge(stack) +"/"+getMaxCharge(stack);

[bdew]

I'm not using the API wrong, i'm doing literally what it says "you can delegate to the default implementations through ElectricItem.rawManager".

when you call the raw then it should be in every case IElectricItem

This is not a requirement anywhere in the API. And is not required in normal IC2.

The fix on your side would be to implement your own tooltip implementation

If you won't fix your side that's what i'll do, but i'm sure there are other IC2 addons that do the same thing (because it's literally what the API says to do), which you won't be compatible with.

[Speiger]

At least I know you love performance drainer loops.

[Speiger]

OK now that this mind is out of the way. Since your whole discharge function is disabled you can return false on canUse &Use because they work with the discharge function. Same counts from charge from armor. The implementation of get tooltip can be done really simply on your side too... Because the only thing you do is litterly calling: ElectricItem.manager.getCharge() even with your RawManager.getCharge() So yeah if you love stupid loops go ahead disable ic2 support if classic is loaded.

[bdew]

Again, i have no problem implementing it myself.

My problem is that i'm doing literally what the API docs say and you claim API compatibility with original IC2 API yet your code isn't actually compatible with it. If you think that API is stupid - then maybe you shouldn't use it.

[Speiger]

Yeah on the other hand IC2Exp is pretty poorly in API design... If you check out how much i added into my side of API/flexibleness to things you see how much you could miss.. So i am supporting the API but only as much as needed. And performance > the 100% doing what they typed.