SpongePowered / SpongeForge

A Forge mod that implements SpongeAPI
http://www.spongepowered.org/
MIT License
1.14k stars 306 forks source link

Farseek update incompatible with Sponge #3041

Closed Xplodin closed 4 years ago

Xplodin commented 4 years ago

I am currently running

https://gist.github.com/Xplodin/5b97ee63b3d46bc564644444cce1fdf4

This worked with other packs on 2847. just not mc.eternal for some reason. Ive applied all foamfix fixes and other config changes but get the same result

clienthax commented 4 years ago

Something is breaking one of our mixins, specifically world.chunk.ChunkMixin_Forge

Caused by: org.spongepowered.asm.mixin.injection.throwables.InjectionError: Critical injection failure: Redirector redirect$forgeImpl$IgnoreLoadEvent$bam000(Lnet/minecraftforge/fml/common/eventhandler/EventBus;Lnet/minecraftforge/fml/common/eventhandler/Event;)Z in mixins.forge.core.json:world.chunk.ChunkMixin_Forge failed injection check, (0/1) succeeded. Using refmap mixins.forge.refmap.json

Can you try without lag goggles, if that doesn't work can you start the server with -Dmixin.debug=true and provide the log.

gabizou commented 4 years ago

Can you add -Dmixin.dumpTargetOnFailure=true and upload the .mixin.out folder after it crashes? There's likely another coremod that is transforming our target (or forge updated their event) and it's causing SpongeForge's target to go missing.

Xplodin commented 4 years ago

EDIT: wrong files. uploading new ones

Xplodin commented 4 years ago

Updated latest.log and .mixin.out.zip

Sorry about that

gabizou commented 4 years ago

TLDR; Farseek does whacky things and Sponge won't support their way of doing these sort of transformations, because cases like this happen.

Based on Farseek doing a wildcard catch all replacement of MinecraftForge.EVENT_BUS.post(...), all expected replacements of the method are being rewritten to Farseek's package, which doesn't quite make sense.

The two changes that Sponge makes are already laid out in the Mixin in question: https://github.com/SpongePowered/SpongeForge/blob/ba8e427d60adaf905d5b40e05650809b9c6401c6/src/main/java/org/spongepowered/mod/mixin/core/world/chunk/ChunkMixin_Forge.java#L60-L74

are meant to avoid throwing duplicated events. The problem here is that Sponge doesn't have a way to avoid these sort of counter modifications (in fact, I'd be sure that Farseek is unintentionally redirecting more calls than they are expecting).

Here's the output of the target for Chunk#onLoad (from the mixin output provided by Xplodin (thank you btw, that nailed down the issue 100%):

// Decompiled java output
   public void func_76631_c() {
      this.field_76636_d = true;
      this.field_76637_e.func_147448_a(this.field_150816_i.values());
      ClassInheritanceMultiMap[] var1 = this.field_76645_j;
      int var2 = var1.length;

      for(int var3 = 0; var3 < var2; ++var3) {
         ClassInheritanceMultiMap classinheritancemultimap = var1[var3];
         this.field_76637_e.func_175650_b(ImmutableList.copyOf(classinheritancemultimap));
      }

     // This is where Sponge would be redirecting first
      package.post(MinecraftForge.EVENT_BUS, new Load(this));
   }

And the bxytecode output:

     public func_76631_c() { //()V
         <localVar:index=4 , name=classinheritancemultimap , desc=Lnet/minecraft/util/ClassInheritanceMultiMap;, sig=Lnet/minecraft/util/ClassInheritanceMultiMap<Lnet/minecraft/entity/Entity;>;, start=L1, end=L2>
         <localVar:index=0 , name=this , desc=Lnet/minecraft/world/chunk/Chunk;, sig=null, start=L3, end=L4>

         L3 {
             aload0 // reference to self
             iconst_1
             putfield net/minecraft/world/chunk/Chunk.field_76636_d:boolean
         }
         L5 {
             aload0 // reference to self
             getfield net/minecraft/world/chunk/Chunk.field_76637_e:net.minecraft.world.World
             aload0 // reference to self
             getfield net/minecraft/world/chunk/Chunk.field_150816_i:java.util.Map
             invokeinterface java/util/Map.values()Ljava/util/Collection;
             invokevirtual net/minecraft/world/World.func_147448_a(Ljava/util/Collection;)V
         }
         L6 {
             aload0 // reference to self
             getfield net/minecraft/world/chunk/Chunk.field_76645_j:net.minecraft.util.ClassInheritanceMultiMap[]
             astore1
             aload1
             arraylength
             istore2
             iconst_0
             istore3
         }
         L7 {
             f_new (Locals[4]: net/minecraft/world/chunk/Chunk, [Lnet/minecraft/util/ClassInheritanceMultiMap;, 1, 1) (Stack[0]: null)
             iload3
             iload2
             if_icmpge L8
             aload1
             iload3
             aaload
             astore4
         }
         L1 {
             aload0 // reference to self
             getfield net/minecraft/world/chunk/Chunk.field_76637_e:net.minecraft.world.World
             aload4
             invokestatic com/google/common/collect/ImmutableList.copyOf(Ljava/util/Collection;)Lcom/google/common/collect/ImmutableList;
             invokevirtual net/minecraft/world/World.func_175650_b(Ljava/util/Collection;)V
         }
         L2 {
             iinc 3 1
             goto L7
         }
         L8 {
             f_new (Locals[1]: net/minecraft/world/chunk/Chunk) (Stack[0]: null)
             getstatic net/minecraftforge/common/MinecraftForge.EVENT_BUS:net.minecraftforge.fml.common.eventhandler.EventBus
             new net/minecraftforge/event/world/ChunkEvent$Load
             dup
             aload0 // reference to self
             invokespecial net/minecraftforge/event/world/ChunkEvent$Load.<init>(Lnet/minecraft/world/chunk/Chunk;)V
             invokestatic farseek/world/gen/structure/package.post(Lnet/minecraftforge/fml/common/eventhandler/EventBus;Lnet/minecraftforge/fml/common/eventhandler/Event;)Z
             pop
         }
         L9 {
             return
         }
         L4 {
         }
     }

I'd be sure that Farseek is unintentionally redirecting more calls than they are expecting

The reason why I say this is that from their transformer, it appears that their transformer is doing a redirect for ALL EventBus.post() calls, no matter where they originate from, and this shows in places that they've no business touching, like:

// Biome.java
   public int getWaterColorMultiplier() {
      GetWaterColor event = new GetWaterColor(this, this.field_76759_H);
      package.post(MinecraftForge.EVENT_BUS, event);
      return event.getNewColor();
   }

   public int getModdedBiomeGrassColor(int original) {
      GetGrassColor event = new GetGrassColor(this, original);
      package.post(MinecraftForge.EVENT_BUS, event);
      return event.getNewColor();
   }

   public int getModdedBiomeFoliageColor(int original) {
      GetFoliageColor event = new GetFoliageColor(this, original);
      package.post(MinecraftForge.EVENT_BUS, event);
      return event.getNewColor();
   }
     public getWaterColorMultiplier() { //()I
         <localVar:index=0 , name=this , desc=Lnet/minecraft/world/biome/Biome;, sig=null, start=L1, end=L2>
         <localVar:index=1 , name=event , desc=Lnet/minecraftforge/event/terraingen/BiomeEvent$GetWaterColor;, sig=null, start=L3, end=L2>

         L1 {
             new net/minecraftforge/event/terraingen/BiomeEvent$GetWaterColor
             dup
             aload0 // reference to self
             aload0 // reference to self
             getfield net/minecraft/world/biome/Biome.field_76759_H:int
             invokespecial net/minecraftforge/event/terraingen/BiomeEvent$GetWaterColor.<init>(Lnet/minecraft/world/biome/Biome;I)V
             astore1
         }
         L3 {
             getstatic net/minecraftforge/common/MinecraftForge.EVENT_BUS:net.minecraftforge.fml.common.eventhandler.EventBus
             aload1
             invokestatic farseek/world/gen/structure/package.post(Lnet/minecraftforge/fml/common/eventhandler/EventBus;Lnet/minecraftforge/fml/common/eventhandler/Event;)Z
             pop
         }
         L4 {
             aload1
             invokevirtual net/minecraftforge/event/terraingen/BiomeEvent$GetWaterColor.getNewColor()I
             ireturn
         }
         L2 {
         }
     }

     public getModdedBiomeGrassColor(int arg0) { //(I)I
         <localVar:index=0 , name=this , desc=Lnet/minecraft/world/biome/Biome;, sig=null, start=L1, end=L2>
         <localVar:index=1 , name=original , desc=I, sig=null, start=L1, end=L2>
         <localVar:index=2 , name=event , desc=Lnet/minecraftforge/event/terraingen/BiomeEvent$GetGrassColor;, sig=null, start=L3, end=L2>

         L1 {
             new net/minecraftforge/event/terraingen/BiomeEvent$GetGrassColor
             dup
             aload0 // reference to self
             iload1
             invokespecial net/minecraftforge/event/terraingen/BiomeEvent$GetGrassColor.<init>(Lnet/minecraft/world/biome/Biome;I)V
             astore2
         }
         L3 {
             getstatic net/minecraftforge/common/MinecraftForge.EVENT_BUS:net.minecraftforge.fml.common.eventhandler.EventBus
             aload2
             invokestatic farseek/world/gen/structure/package.post(Lnet/minecraftforge/fml/common/eventhandler/EventBus;Lnet/minecraftforge/fml/common/eventhandler/Event;)Z
             pop
         }
         L4 {
             aload2
             invokevirtual net/minecraftforge/event/terraingen/BiomeEvent$GetGrassColor.getNewColor()I
             ireturn
         }
         L2 {
         }
     }

     public getModdedBiomeFoliageColor(int arg0) { //(I)I
         <localVar:index=0 , name=this , desc=Lnet/minecraft/world/biome/Biome;, sig=null, start=L1, end=L2>
         <localVar:index=1 , name=original , desc=I, sig=null, start=L1, end=L2>
         <localVar:index=2 , name=event , desc=Lnet/minecraftforge/event/terraingen/BiomeEvent$GetFoliageColor;, sig=null, start=L3, end=L2>

         L1 {
             new net/minecraftforge/event/terraingen/BiomeEvent$GetFoliageColor
             dup
             aload0 // reference to self
             iload1
             invokespecial net/minecraftforge/event/terraingen/BiomeEvent$GetFoliageColor.<init>(Lnet/minecraft/world/biome/Biome;I)V
             astore2
         }
         L3 {
             getstatic net/minecraftforge/common/MinecraftForge.EVENT_BUS:net.minecraftforge.fml.common.eventhandler.EventBus
             aload2
             invokestatic farseek/world/gen/structure/package.post(Lnet/minecraftforge/fml/common/eventhandler/EventBus;Lnet/minecraftforge/fml/common/eventhandler/Event;)Z
             pop
         }
         L4 {
             aload2
             invokevirtual net/minecraftforge/event/terraingen/BiomeEvent$GetFoliageColor.getNewColor()I
             ireturn
         }
         L2 {
         }
     }

Since it's been a known transformative issue of how Farseek goes about modifying the game (and Sponge does it's best to guarantee compatibility in a wide variety of configurations), I'll close with a short list of previous issues outlined by Farseek:

https://github.com/SpongePowered/SpongeForge/issues/754 https://github.com/delvr/Farseek/issues/16