MinecraftForge / ForgeGradle

Minecraft mod development framework used by Forge and FML for the gradle build system
GNU Lesser General Public License v2.1
514 stars 443 forks source link

FG_3.0 Extra Parameter Annotation being added to TextFormatting.class #530

Closed modmuss50 closed 5 years ago

modmuss50 commented 5 years ago

Ive been messing with 1.13 forge, and found an issue when trying to build a mod that uses TextFormatting.class

Java compiler output when running gradlew.bat build: https://gist.github.com/modmuss50/54677952de67d2e752456506875d28c8

This seems to be caused by an extra @Nullable paramter being added when it shouldn't be. (seen using idea's bytecode viewer)

idea64_2018-11-19_19-36-24

This can be reproduced by cloning and importing the lastest 1.13 test MDK into idea and adding the following line

LOGGER.info(TextFormatting.RED); into the example mod, any use of the text formatting class seems to cause this issue.

Let me know if you need anymore info.

Pokechu22 commented 5 years ago

Ooohh... This might very well be my fault, or it might be a case I didn't handle. See #472 and ModCoderPack/MCInjector#3. What's going on is that there are synthetic parameters for enums (and inner classes), and nothing really knows what to do with that in terms of annotations. My rough guess, though, is that I'm assuming the synthetic parameters are always present when they aren't in some cases (such as, say, when an enum has multiple constructors).

I'll investigate; my guess is that the enum edge case is a thing, though if so I don't know how this didn't cause problems before. In any case: there is actually an annotation, it's just that the index on it is probably being changed in a way that it shouldn't be to compensate for other things changing it.

Pokechu22 commented 5 years ago

OK, looks like it's just a case I thought of before but didn't test because it didn't exist then; in the MCInjector pull request I mentioned it though:

There are no enums with constructors that have annotations on them.

However, it seems like it's not actually a MCInjector problem. A walk through of what it looks like through different parts of the toolchain:

Here's an annotated version of the relevant output from javap on the obfuscated jar (conveniently, TextFormatting is a):

Click to expand
  private a(java.lang.String, char, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #377                         // (Ljava/lang/String;CILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
      parameter 5:
        0: #49() // Ljavax/annotation/Nullable;
  private a(java.lang.String, char, boolean);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZ)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #379                         // (Ljava/lang/String;CZ)V
  private a(java.lang.String, char, boolean, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #380                         // (Ljava/lang/String;CZILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
      parameter 5:
      parameter 6:
        0: #49() // Ljavax/annotation/Nullable;

This is what it looks like in 1.13.2.joined.mapped.jar (note that it hasn't changed yet, as mcinjector didn't run):

Click to expand
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #93                          // (Ljava/lang/String;CILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
      parameter 5:
        0: #51() // Ljavax/annotation/Nullable;
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZ)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #99                          // (Ljava/lang/String;CZ)V
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #100                         // (Ljava/lang/String;CZILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
      parameter 5:
      parameter 6:
        0: #51() // Ljavax/annotation/Nullable;

And here's what it looks like in 1.13.2.joined.mci.jar; note that the extra parameters are gone:

Click to expand
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #94                          // (Ljava/lang/String;CILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
        0: #51() // Ljavax/annotation/Nullable;
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZ)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #106                         // (Ljava/lang/String;CZ)V
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #112                         // (Ljava/lang/String;CZILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
        0: #51() // Ljavax/annotation/Nullable;

Additionally, if I take the decompiled TextFormatting from 1.13.2.joined.decomp.jar and compile it with javac -cp 1.13.2.joined.mci.jar 1.13.2.server.jar TextFormatting.java (the server jar is referenced only so that I have libraries; it's lazy but it works), and then disassemble the result, I get this (which matches the one from MCInjector):

Click to expand
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #172                         // (Ljava/lang/String;CILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
        0: #159() // Ljavax/annotation/Nullable;
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZ)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #175                         // (Ljava/lang/String;CZ)V
  private net.minecraft.util.text.TextFormatting(java.lang.String, char, boolean, int, java.lang.Integer);
    descriptor: (Ljava/lang/String;ILjava/lang/String;CZILjava/lang/Integer;)V
    flags: ACC_PRIVATE
    Code:
      // -snip-
    Signature: #177                         // (Ljava/lang/String;CZILjava/lang/Integer;)V
    RuntimeVisibleParameterAnnotations:
      parameter 0:
      parameter 1:
      parameter 2:
      parameter 3:
      parameter 4:
        0: #159() // Ljavax/annotation/Nullable;

 

So... the MCInjector code is working fine, and it's fine when it's called by MCPConfig. There must be some other issue with how ForgeGradle 3 is calling MCPConfig, then; I don't know what it is exactly and I haven't messed with FG3 enough to know how to diagnose it.

modmuss50 commented 5 years ago

This is fixed. Thanks