[1.12.2] TileController.getWorld() crash loop
lightlike opened this issue · 10 comments
Issue description:
Same as #2241
Steps to reproduce:
- Remove Controller from Overworld
- Place Controller in Dimensional Doors Personal Dimension (Previously connected via Remote network transmitter/reciever)
- Crash
Version (make sure you are on the latest version before reporting):
- Minecraft: 1.12.2
- Forge: 14.23.5.2847
- Refined Storage: 1.6.16
Does this issue occur on a server? [yes/no]
yes
Crash-Report
@ian-rampage any ideas?
I'll have a peek at the built artifact next weekend. If it's still happening it might be a quirk in the mc build methods (goodness knows it has some...). I hope I can fix it simply, but might need to end up with some duplication...
OK. This one is a bit of a puzzler. There is at least one bug in the obfuscation going on here.
I'm going to take a break from this and come back from the perspective of avoiding these collisions, rather than fixing them in place. Sorry if this leaves a bit of mess, but I'm not good enough with the Minecraft ecosystem to fix this properly.
Post obs:
TileController.java:
public net.minecraft.world.World func_145831_w();
descriptor: ()Lnet/minecraft/world/World;
flags: ACC_PUBLIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: getfield #293 // Field field_145850_b:Lnet/minecraft/world/World;
4: areturn
LocalVariableTable:
Start Length Slot Name Signature
0 5 0 this Lcom/raoulvdberge/refinedstorage/tile/TileController;
LineNumberTable:
line 518: 0
API.java
public boolean isNetworkNodeEqual(com.raoulvdberge.refinedstorage.api.network.node.INetworkNode, java.lang.Object);
descriptor: (Lcom/raoulvdberge/refinedstorage/api/network/node/INetworkNode;Ljava/lang/Object;)Z
flags: ACC_PUBLIC
Code:
stack=2, locals=4, args_size=3
0: aload_2
1: instanceof #497 // class com/raoulvdberge/refinedstorage/api/network/node/INetworkNode
4: ifne 9
7: iconst_0
8: ireturn
9: aload_1
10: aload_2
11: if_acmpne 16
14: iconst_1
15: ireturn
16: aload_2
17: checkcast #497 // class com/raoulvdberge/refinedstorage/api/network/node/INetworkNode
20: astore_3
21: aload_1
22: invokeinterface #612, 1 // InterfaceMethod com/raoulvdberge/refinedstorage/api/network/node/INetworkNode.getWorld:()Lnet/minecraft/world/World;
27: getfield #616 // Field net/minecraft/world/World.field_73011_w:Lnet/minecraft/world/WorldProvider;
30: invokevirtual #621 // Method net/minecraft/world/WorldProvider.getDimension:()I
33: aload_3
34: invokeinterface #612, 1 // InterfaceMethod com/raoulvdberge/refinedstorage/api/network/node/INetworkNode.getWorld:()Lnet/minecraft/world/World;
39: getfield #616 // Field net/minecraft/world/World.field_73011_w:Lnet/minecraft/world/WorldProvider;
42: invokevirtual #621 // Method net/minecraft/world/WorldProvider.getDimension:()I
45: if_icmpeq 50
48: iconst_0
49: ireturn
50: aload_1
51: invokeinterface #608, 1 // InterfaceMethod com/raoulvdberge/refinedstorage/api/network/node/INetworkNode.getPos:()Lnet/minecraft/util/math/BlockPos;
56: aload_3
57: invokeinterface #608, 1 // InterfaceMethod com/raoulvdberge/refinedstorage/api/network/node/INetworkNode.getPos:()Lnet/minecraft/util/math/BlockPos;
62: invokevirtual #626 // Method net/minecraft/util/math/BlockPos.equals:(Ljava/lang/Object;)Z
65: ireturn
LocalVariableTable:
Start Length Slot Name Signature
0 66 0 this Lcom/raoulvdberge/refinedstorage/apiimpl/API;
0 66 1 left Lcom/raoulvdberge/refinedstorage/api/network/node/INetworkNode;
0 66 2 right Ljava/lang/Object;
21 45 3 rightNode Lcom/raoulvdberge/refinedstorage/api/network/node/INetworkNode;
LineNumberTable:
line 365: 0
line 366: 7
line 369: 9
line 370: 14
line 373: 16
line 375: 21
line 376: 48
line 379: 50
StackMapTable: number_of_entries = 3
frame_type = 9 /* same */
frame_type = 6 /* same */
frame_type = 252 /* append */
offset_delta = 33
locals = [ class com/raoulvdberge/refinedstorage/api/network/node/INetworkNode ]
And looking through the replacement logs:
Line 16532: MD: net/minecraft/block/BlockSourceImpl/getWorld ()Lnet/minecraft/world/World; net/minecraft/block/BlockSourceImpl/func_82618_k ()Lnet/minecraft/world/World;
Line 16613: MD: net/minecraft/dispenser/ILocation/getWorld ()Lnet/minecraft/world/World; net/minecraft/dispenser/ILocation/func_82618_k ()Lnet/minecraft/world/World;
Line 22020: MD: net/minecraft/entity/IEntityMultiPart/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/IEntityMultiPart/func_82194_d ()Lnet/minecraft/world/World;
Line 22046: MD: net/minecraft/entity/boss/EntityDragon/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/boss/EntityDragon/func_82194_d ()Lnet/minecraft/world/World;
Line 23223: MD: net/minecraft/entity/NpcMerchant/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/NpcMerchant/func_190670_t_ ()Lnet/minecraft/world/World;
Line 23288: MD: net/minecraft/entity/passive/EntityVillager/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/passive/EntityVillager/func_190670_t_ ()Lnet/minecraft/world/World;
Line 23911: MD: net/minecraft/entity/item/EntityMinecartHopper/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/item/EntityMinecartHopper/func_145831_w ()Lnet/minecraft/world/World;
Line 25095: MD: net/minecraft/entity/IMerchant/getWorld ()Lnet/minecraft/world/World; net/minecraft/entity/IMerchant/func_190670_t_ ()Lnet/minecraft/world/World;
Line 28048: MD: net/minecraft/tileentity/TileEntity/getWorld ()Lnet/minecraft/world/World; net/minecraft/tileentity/TileEntity/func_145831_w ()Lnet/minecraft/world/World;
Line 28230: MD: net/minecraft/tileentity/IHopper/getWorld ()Lnet/minecraft/world/World; net/minecraft/tileentity/IHopper/func_145831_w ()Lnet/minecraft/world/World;
Line 28945: MD: net/minecraft/world/chunk/Chunk/getWorld ()Lnet/minecraft/world/World; net/minecraft/world/chunk/Chunk/func_177412_p ()Lnet/minecraft/world/World;
Line 33957: MD: net/minecraft/client/renderer/tileentity/TileEntitySpecialRenderer/getWorld ()Lnet/minecraft/world/World; net/minecraft/client/renderer/tileentity/TileEntitySpecialRenderer/func_178459_a ()Lnet/minecraft/world/World;
Line 34092: MD: net/minecraft/client/renderer/chunk/RenderChunk/getWorld ()Lnet/minecraft/world/World; net/minecraft/client/renderer/chunk/RenderChunk/func_188283_p ()Lnet/minecraft/world/World;
Line 35964: MD: net/minecraft/server/MinecraftServer/getWorld (I)Lnet/minecraft/world/WorldServer; net/minecraft/server/MinecraftServer/func_71218_a (I)Lnet/minecraft/world/WorldServer;
So - after far too much time pratting about learning gradle - I can say that what is basically happening is this:
The obfuscation setup used in minecraft is not just based on signitures - but also on location.
As such, a getWorld() on one interface may not be the same method (post obfuscation) as a getWorld on annother method.
Thus the reobfuscator has to obfuscate on a methood based on interfaces that the given method impliments.
As such, the method getWorld on TileController will be renamed to the obfuscated version from TileEntity (func_145831_w)
However, the getWorld method on INetworkNode is more complex. This could need to be func_71218_a in ne place, func_178459_a in annother and func_188283_p and at the point it is compiling, it dosent know which to pick, so it does none of them.
This leaves us with three options:
Firstly, we could pursuade Mojang to use a sane obfuscator to better support the modding community (Im aware this wont happen - I need to vent slightly).
Secondly we could try and dig deeper into the reobs step of the build - I havent been able to figure it out yet, but it looks like there are ways to add custom deobfuscators in - but from a quick search there dosent appear t be a way oof doing this that isent hacky and fragile - but I havent fully explored this yet.
Finally, we could comb through the code and look for places where we override or utilise one of the 20K fields or 20K methods that are obfuscated, and ensure that it is not also filling a secondary interface.
So in short -
Screw the obfuscator in perticular
If you want me t fix this, it will end up being an ugly fix with some number of duplicated records.
If anybody has worked on the deobs task in GradleForge before - can you help us fix this in a cleaner manner (and ideally automate this for everyone)?
Ill try and make a minimal example to send to GradleForge later.
See you around!
I've written a small program to find all the abstract calls after obfuscation.
If anyone knows the build process enough to try and run it as part of that, please let me know, as I cant figure it out.
Obviously the problem is fixable without doing this, but we obviously dont want this to regress.
I have an application which simply needs to run with the following classPath (sorry for the local downloads - they are the obfuscated/released versions of the mods):
jar://$MODULE_DIR$/../../.gradle/minecraft/forgeBin-1.12.2-14.23.4.2760-PROJECT(refinedstorage).jar
jar://$USER_HOME$/Downloads/funky-locomotion-1.12.2-1.1.2.jar
jar://$USER_HOME$/Downloads/OpenComputers-MC1.12.2-1.7.5.192.jar
Gradle: io.netty:netty-all:4.1.9.Final
Gradle: org.apache.commons:commons-lang3:3.5
Gradle: com.google.guava:guava:18.0
Gradle: net.sf.trove4j:trove4j:3.0.3
jar://$MODULE_DIR$/../../build/libs/refinedstorage-1.6.12.jar
And should be run after the build() step (i.e. refinedstorage-1.6.12.jar is obfuscated).
The current output of this is:
ABSTRACT METHOD CALL: Class: com.raoulvdberge.refinedstorage.tile.TileController, Interface: INetworkNode, Method: public abstract void com.raoulvdberge.refinedstorage.api.network.node.INetworkNode.update()
ABSTRACT METHOD CALL: Class: com.raoulvdberge.refinedstorage.tile.TileController, Interface: INetworkNode, Method: public abstract net.minecraft.world.World com.raoulvdberge.refinedstorage.api.network.node.INetworkNode.getWorld()
ABSTRACT METHOD CALL: Class: com.raoulvdberge.refinedstorage.tile.TileController, Interface: INetworkNode, Method: public abstract void com.raoulvdberge.refinedstorage.api.network.node.INetworkNode.markDirty()
ABSTRACT METHOD CALL: Class: com.raoulvdberge.refinedstorage.tile.TileController, Interface: INetworkNode, Method: public abstract net.minecraft.util.math.BlockPos com.raoulvdberge.refinedstorage.api.network.node.INetworkNode.getPos()
So it looks like there are only four methods that can call this, and they are all related to INetworkNode.
I am however slightly concerned about one of these @raoulvdberge:
INetworkNode#markDirty() has the impact of running markForSaving() on the active NetworkNodeManager for every instance except for TileController - is this expected to be a special case - previously we would get this crash - keen to make sure we are not walking into another issue.
If so I should be able to raise a PR for this relatively soon.
Thanks for all the troubleshooting. Can we do some quickfixes, like renaming methods? Going deep into the Forge obfuscation stuff doesn't seem productive to me.