AgriCraft

AgriCraft

30M Downloads

Shift right clicking cropsticks to place the cross sticks crashes server

scalda opened this issue ยท 8 comments

commented

Hi,

I have just updated agricraft and infiniraider

latest version
agricraft-2.0.0-0.10.0-a18.jar
infinitylib-0.10.0.jar

Whenever I place cross sticks via shift right clicking on tilled soil it crashes the server
crahs report

regards

Scalda
(FoolCraft Modpack Dev)

commented

Yay! Fatal error in fancy new code... And I thought I had gotten better at this...

commented

Wait... Wat... How can you get an abstract method error at runtime? Something seriously hinky is going on here... I'm not even sure if that can be debugged...

Uh @TehNut HALP!

commented

I guess I could rename the method and provide an implementation in the class. The dispatch doesn't make any sense though... And why is this issue just showing up now?

commented

I'm not sure if it's related, but I have the same versions and I crash when I try to use Extra Utilities on cross crop sticks as well as when I right click a set of cross crops.

commented

This is a very weird bug, yup. It hasn't happened to me yet, but I also haven't tried using Actually Additions plus LordDan1989's instructions in #997 to recreate it.

As for the timing, well the May 24th commit (e7c4f0e) changed SpreadStrategy (L27) and MutateStrategy to call crop.getWorld(), and also added getWorld() to the IAgriCrop interface (L37). The a17 release was before that, and a18 after that.

A lot of the help for AbstractMethodErrors blames bytecode that was cached instead of recompiled properly to reflect some new source code, but since TileEntityCrop didn't have a getWorld method removed, that doesn't make sense. Hm.

Also: how about just adding the method to TileEntityCrop.java? (Instead of renaming it.)

@Override
public World getWorld() {
    return super.getWorld(); // Or TileEntity.getWorld()
}
Less related results
More testing and searching
// First find files with both names: 
find . -type f -exec grep -qi 'iagricrop' {} \; -exec grep -il 'getworld' {} +

// Then see how they're each used: 
egrep -ni1 "iagricrop|getworld" ./tiles/TileEntityCrop.java ./handler/PlayerInteractEventHandler.java ./api/v1/crop/IAgriCrop.java ./farming/mutation/MutateSt
rategy.java ./farming/mutation/SpreadStrategy.java 

// And this shows us that the only place where IAgriCrop's getWorld is called is in the strategies. 
// That could explain why this error hasn't occurred before, because this situation is maybe unique. 

Think-I-found-an-answer edit:

The re-obfuscation process might be to blame.

The solution in these and related discussions is to create bridge methods slash put redirects in the child classes.

So, maybe like the code above? The reason it works is that the name+reference for the TileEntityCrop.getWorld() stub method matches the IAgriCrop interface, while the reference to TileEntity.getWorld() gets properly re-obfuscated and unbroken. The error happens because the obfuscated getWorld doesn't match the signature for IAgriCrop. Haha! (Also, I get now what RlonRyan meant by renaming it. Took me too long to catch up.)

commented

I'm tidying up my research tabs, and since it felt like the discussions on these pages might be relevant, I wanted to share them.

  • Oracle, bug report about calling inherited methods from abstract classes in separate packages. Hard to say if this is similar enough.
  • Artima, see Static versus Dynamic Binding. I thought this was going to be helpful, since the liq.gurgle() example is about unexpected method resolution. But really the bug in agricraft isn't one of the three exceptional cases they list.
  • StackOverflow, confirms that a child can fulfill an interface using methods inherited from an abstract parent. So that's indeed supposed to work fine.
  • Apprize, focuses on default methods in interfaces, but does discuss well the resolution rules.
commented

Yup, I think I've found the answer.

CovertJaguar commented on Jun 11, 2016
Also the infamous "getWorld()" bugs. ;)
md-5/SpecialSource#12 (comment)

Update:

Although it turns out that the interface methods have to be named differently*, I tested out the fix and confirmed that it removed the AbstractMethodError's. I was able to get several plants to spread to cross crops. So, I uploaded my edit as a PR.

Screenshot: (more in album: http://imgur.com/a/MvBJ6)

abstractmethoderror testing 05

commented

Well this bug is fixed thanks to @CodesCubesAndCrashes... I'm still leaving the wat label applied to it though, because, seriously, wat Forge? Wat?