[FG-2.3] Update to MCInjector 1.6

[Pokechu22]

Fixes #472.

Code diff (1.12.2):

diff --git a/projects/src/main/java/net/minecraft/client/renderer/RenderGlobal.java b/projects/src/main/java/net/minecraft/client/renderer/RenderGlobal.java
index 821cf8a..2a20535 100644
--- a/projects/src/main/java/net/minecraft/client/renderer/RenderGlobal.java
+++ b/projects/src/main/java/net/minecraft/client/renderer/RenderGlobal.java
@@ -2262,7 +2262,7 @@ public class RenderGlobal implements IWorldEventListener, IResourceManagerReload
         byte setFacing;
         final int counter;
 
-        private ContainerLocalRenderInformation(RenderChunk renderChunkIn, EnumFacing facingIn, @Nullable int counterIn) {
+        private ContainerLocalRenderInformation(RenderChunk renderChunkIn, @Nullable EnumFacing facingIn, int counterIn) {
             this.renderChunk = renderChunkIn;
             this.facing = facingIn;
             this.counter = counterIn;
diff --git a/projects/src/main/java/net/minecraft/network/play/server/SPacketPlayerListItem.java b/projects/src/main/java/net/minecraft/network/play/server/SPacketPlayerListItem.java
index d4fec5c..4839a72 100644
--- a/projects/src/main/java/net/minecraft/network/play/server/SPacketPlayerListItem.java
+++ b/projects/src/main/java/net/minecraft/network/play/server/SPacketPlayerListItem.java
@@ -198,7 +198,7 @@ public class SPacketPlayerListItem implements Packet<INetHandlerPlayClient> {
         private final GameProfile profile;
         private final ITextComponent displayName;
 
-        public AddPlayerData(GameProfile profileIn, int latencyIn, GameType gameModeIn, @Nullable ITextComponent displayNameIn) {
+        public AddPlayerData(GameProfile profileIn, int latencyIn, @Nullable GameType gameModeIn, @Nullable ITextComponent displayNameIn) {
             this.profile = profileIn;
             this.ping = latencyIn;
             this.gamemode = gameModeIn;

This does switch to asm 6.1 from 6.0 (I think), which seems to cause other changes (among others, it looks like the remapped jar has some attributes in a different order, causing a lot of class files with different hashes). These shouldn't cause issues though, I think; only the above changes are present after decompiling.

[LexManos]

Have you confirmed that these do not effect Forge's patches for MC versions that use FG 2.3?

[Pokechu22]

Tested:

• Ran cleanCache and then compiled schematica (commit 0285264) with only setupDevWorkspace and runClient; everything ran fine.
• Compiled forge 1.12.2 (commit 34f4381) after cleanCache (and clean); build and runClient went fine.

From a logical standpoint, I don't think it should affect anything, since forge's binary patches are applied before the deobf process (including mcinjector), at least according to the task order in the build log. (Clarified elsewhere that it was the source patches to check, currently doing that now)

[Pokechu22]

Forge switched to FG 2.3 on commit f1cca47 (1.12.0), found via git log -p -G"ForgeGradle" build.gradle.

As per git log -p f1cca47~..HEAD -- patches/minecraft/net/minecraft/network/play/server/SPacketPlayerListItem.java.patch patches/minecraft/net/minecraft/client/renderer/RenderGlobal.java.patch:

• There are no revisions that touch SPacketPlayerListItem.
• There are 4 versions of the patch that touch RenderGlobal. None of them touch the constructor to ContainerLocalRenderInformation, and the constructor is not included in the patch at all; while the constructor is referenced only the annotations have changed so that will not affect compilation. The fix did not change the number of lines, so patches should continue to apply cleanly.

As an extra check, git whatchanged reports no revisions that ever touched a file with SPacketPlayerListItem in the name.

[AEnterprise]

this did break forge's patches, the setupCiWorkspace task now fails on net.minecraft.client.renderer.entity.layers.LayerHeldItem (ccc)

tried with both 14.23.0.2544 (that we are using) and 14.23.3.2670 (latest, build after this PR was merged)

[Pokechu22]

Confirmed... hm. Will test further. Specifically output is:

:applyBinaryPatches FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':applyBinaryPatches'.
> There is a binary discrepency between the expected input class net.minecraft.client.renderer.entity.layers.LayerHeldItem (ccc) and the actual class. Checksum on disk is da0f6687, in patch 5f7c661a. Things are probably about to go very wrong. Did you put something into the jar file?

(edit: for reference:

git clone https://github.com/BuildCraft/BuildCraft
cd BuildCraft
./gradlew setupCiWorkspace build

)

[Pokechu22]

I didn't consider the mergeJars task, which also uses ASM. That's the cause :/

And the unit test for mergeJars didn't catch it, because it seems to only check if the names of the files match and not the actual binary :/

[mezz]

@Pokechu22 do you have a fix in mind or is there something else we should look into? I can wait for a PR if you are already looking into it.

[Pokechu22]

My current plan is to see if it's possible to run mergeJars with a different asm version, but I don't have an exact fix planned yet and I'm not sure how quickly I'd be able to do it. So I think it might make sense to revert temporarily while I'm working on it and then reapply this and the fix once Ilm done.

[mezz]

Sounds good, thanks.

[bs2609]

Linking ModCoderPack/MCInjector#4 for reference.