Skip to content

Remap MCP Mappings#1506

Merged
LAGIdiot merged 3 commits intoGregTechCE:masterfrom
serenibyss:mcpmapping
Apr 7, 2021
Merged

Remap MCP Mappings#1506
LAGIdiot merged 3 commits intoGregTechCE:masterfrom
serenibyss:mcpmapping

Conversation

@serenibyss
Copy link
Copy Markdown
Collaborator

@serenibyss serenibyss commented Feb 25, 2021

What:
This PR addresses the issue discussed in Discord earlier,

We ran into a problem with the JEI OrePage where it would crash sometimes, and not consistently. I found out that it was due to the fact that Biome#biomeName changed at some point from public to private access, and an IllegalAccessException was being thrown due to it being private.
As a result, we think the safest way forward is to update our MCP mappings from snapshot_20170928 to stable_39. I believe that I have fixed all mapping changes in our files (a total of 65 files changed), but I am still working on fixing our build.gradle so that all dependencies compile and run correctly in dev environment.

How solved:
I updated our MCP mappings to stable_39. Notable method name changes:

Block:

  • getBlockLayer() -> getRenderLayer()
  • onBlockDestroyedByPlayer() -> onPlayerDestroy()
  • onEntityCollidedWithBlock() -> onEntityCollision()
  • getMobilityFlag() -> getPushReaction()

World::

  • getChunkFromChunkCoords() -> getChunk()

ResourceLocation:

  • getResourceName() -> getNamespace()
  • getResourcePath() -> getPath()

NBTCompound:

  • hasNoTags() -> isEmpty()

Translation:

  • setUnlocalizedName() -> setTranslationKey()
  • getUnlocalizedName() -> getTranslationKey()

EnumFacing:

  • getFrontOffsetX() -> getXOffset()
  • getFrontOffsetY() -> getYOffset()
  • getFrontOffsetZ() -> getZOffset()

CreativeTabs:

  • getTabIconItem() -> createIcon()

EnumRarity:

  • rarityColor -> color

Biome:

  • biomeName is private, so I changed its references to getBiomeName()

Outcome:
Update MCP Mapping to stable_39

You can verify my work using the official MCP Mapping names website, which is what I used to guarantee that we switch to the correct new method names.

Additional info:
I have not yet gotten the dev environment working without a crash due to dependencies deobfuscated files being outdated. I know that setting this up is possible, as both SoG and Gregicality are already on stable_39, and have successfully remapped SRG->MCP for all of their dependents (same as ours in dev, and more).

Possible compatibility issue:
There may be some work needed for developers to refresh their environments, but I am not sure of the extent of it since I haven't finished the build.gradle

@serenibyss serenibyss marked this pull request as draft February 25, 2021 21:05
@serenibyss
Copy link
Copy Markdown
Collaborator Author

If this PR is not included in next release, we should change biomeName to getBiomeName() for the JEI ore page to at least fix the crash issue present in 1.12.0

@LAGIdiot
Copy link
Copy Markdown
Member

How far did you get with this? Does it work for you I run into 3 problems so far.

  1. LargeStackSizeItemStackHandler there is problem with NBT list
  2. BakedModelHandler there is problem with ItemOverrideList
  3. Game crashes when on
    java.lang.NoSuchMethodError: net.minecraft.util.ResourceLocation.getResourcePath()Ljava/lang/String; at codechicken.lib.model.loader.cube.CCCubeLoader.accepts(CCCubeLoader.java:21) at net.minecraftforge.client.model.ModelLoaderRegistry.getModel(ModelLoaderRegistry.java:123) at net.minecraftforge.client.model.ModelLoader.registerVariant(ModelLoader.java:235) at net.minecraft.client.renderer.block.model.ModelBakery.loadBlock(ModelBakery.java:153) at net.minecraftforge.client.model.ModelLoader.loadBlocks(ModelLoader.java:223) at net.minecraftforge.client.model.ModelLoader.setupModelRegistry(ModelLoader.java:150) at net.minecraft.client.renderer.block.model.ModelManager.onResourceManagerReload(ModelManager.java:28) at net.minecraft.client.resources.SimpleReloadableResourceManager.registerReloadListener(SimpleReloadableResourceManager.java:132) at net.minecraft.client.Minecraft.init(Minecraft.java:560) at net.minecraft.client.Minecraft.run(Minecraft.java:422) at net.minecraft.client.main.Main.main(Main.java:118) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) at net.minecraft.launchwrapper.Launch.main(Launch.java:28) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97) at GradleStart.main(GradleStart.java:25)
    I was able to fix 1 and 2 (for 1 I would need to test it) but I got stuck on 3 as it is not in our code. I am I doing something wrong or?

@serenibyss
Copy link
Copy Markdown
Collaborator Author

The first two, I must have just missed. For issue 3, this is why this is a draft PR, because the issue is that CCL is not using stable_39 since it has not been updated nearly as recently, so it is erroring from that. I know that this issue is possible to resolve, as both Gregicality and SoG use stable_39 themselves and have no issues with GTCE or CCL not using the same mapping. However, I do not know how to resolve it, and as far as I know, it should just be done for us automatically by gradle.

@LAGIdiot
Copy link
Copy Markdown
Member

I know that 3 is doable and I saw it working on Gregicality but I am not sure why it does not work on our project.
I have tried monkeying with Gradle, using same source for CodeChickenLib as Gregicality, wiping caches and rebuilding but nothing.

@warjort
Copy link
Copy Markdown
Contributor

warjort commented Mar 20, 2021

Gregicality is using different maven names for some of chickenbones's mods

try this:

-    "deobfCompile"("codechicken:CodeChickenLib:$mcVersion-$cclVersion:universal")
-    "deobfCompile"("codechicken:ForgeMultipart:$mcVersion-$multipartVersion:universal")
+    "deobfCompile"("codechicken-lib-1-8:CodeChickenLib-1.12.2:3.2.3.358:universal")
+    "deobfCompile"("forge-multipart-cbe:ForgeMultipart:$mcVersion-$multipartVersion:universal")

It looks like codechicken-lib-1.8 is only available from https://minecraft.curseforge.com/api/maven
I don't know where forge-multipart is coming from, but I did manage to download it at some point?

I get a different error now:

[21:22:40] [Client thread/ERROR] [gregtech]: Tried to register recipe, gregtech_color_White, with duplicate key. Recipe: "XXX", "XDX", "XXX", "X", "1xtile.frame@32767", "D", "dyeWhite"
[21:22:40] [Client thread/ERROR] [gregtech]: Stacktrace:
java.lang.IllegalArgumentException: null
        at gregtech.api.recipes.ModHandler.validateRecipe(ModHandler.java:311) ~[ModHandler.class:?]
        at gregtech.api.recipes.ModHandler.addShapedRecipe(ModHandler.java:250) ~[ModHandler.class:?]
        at gregtech.loaders.recipe.CraftingRecipeLoader.registerColoringRecipes(CraftingRecipeLoader.java:242) ~[CraftingRecipeLoader.class:?]
        at java.util.HashMap$Values.forEach(HashMap.java:981) [?:1.8.0_282]
        at gregtech.loaders.recipe.CraftingRecipeLoader.loadCraftingRecipes(CraftingRecipeLoader.java:213) [CraftingRecipeLoader.class:?]
        at gregtech.loaders.recipe.CraftingRecipeLoader.init(CraftingRecipeLoader.java:38) [CraftingRecipeLoader.class:?]
        at gregtech.common.CommonProxy.registerRecipes(CommonProxy.java:153) [CommonProxy.class:?]

I don't understand what is going on with those chickenbones libs.
The one from his maven doesn't seem to work. The differently named one (but same version) from curseforge does?

I tried deleting my cached version of forge-multipart to see if I could figure out where it downloads it from, but now I can't find where that is. I've always liked maven. :-P

@warjort
Copy link
Copy Markdown
Contributor

warjort commented Mar 20, 2021

I managed to figure it out, it is also coming from curseforge.
Somehow I managed to change it so it was a mixture of the old and new names?

The correct name is:

    "deobfCompile"("forge-multipart-cbe:ForgeMultipart-$mcVersion:$multipartVersion:universal")

@warjort
Copy link
Copy Markdown
Contributor

warjort commented Mar 20, 2021

The error above is because of a typo:

diff --git a/src/main/java/gregtech/loaders/recipe/CraftingRecipeLoader.java b/src/main/java/gregtech/loaders/recipe/CraftingRecipeLoader.java
index ab8643ec..3f8e357c 100644
--- a/src/main/java/gregtech/loaders/recipe/CraftingRecipeLoader.java
+++ b/src/main/java/gregtech/loaders/recipe/CraftingRecipeLoader.java
@@ -238,7 +238,7 @@ public class CraftingRecipeLoader {

     private static void registerColoringRecipes(BlockColored block) {
         for (EnumDyeColor dyeColor : EnumDyeColor.values()) {
-            String recipeName = String.format("%s_color_%s", block.getRegistryName().getNamespace(), getColorName(dyeColor));
+            String recipeName = String.format("%s_color_%s", block.getRegistryName().getPath(), getColorName(dyeColor));
             ModHandler.addShapedRecipe(recipeName, new ItemStack(block, 8, dyeColor.getMetadata()), "XXX", "XDX", "XXX",
                 'X', new ItemStack(block, 1, GTValues.W), 'D', getOrdictColorName(dyeColor));
         }

There is also this one:

[23:42:16] [Client thread/ERROR] [gregtech]: Tried to register recipe, forestry_saw, with duplicate key. Recipe: "s", "L", "L", "1xtile.for.logs.0@1"
[23:42:16] [Client thread/ERROR] [gregtech]: Stacktrace:
java.lang.IllegalArgumentException: null
        at gregtech.api.recipes.ModHandler.validateRecipe(ModHandler.java:311) [ModHandler.class:?]
        at gregtech.api.recipes.ModHandler.addShapedRecipe(ModHandler.java:250) [ModHandler.class:?]
        at gregtech.loaders.recipe.WoodMachineRecipes.processLogOreDictionary(WoodMachineRecipes.java:97) [WoodMachineRecipes.class:?]
        at gregtech.loaders.recipe.WoodMachineRecipes.postInit(WoodMachineRecipes.java:28) [WoodMachineRecipes.class:?]
        at gregtech.common.CommonProxy.onPostLoad(CommonProxy.java:242) [CommonProxy.class:?]
        at gregtech.common.ClientProxy.onPostLoad(ClientProxy.java:125) [ClientProxy.class:?]
diff --git a/src/main/java/gregtech/loaders/recipe/WoodMachineRecipes.java b/src/main/java/gregtech/loaders/recipe/WoodMachineRecipes.java
index 55bf705d..c1852f85 100644
--- a/src/main/java/gregtech/loaders/recipe/WoodMachineRecipes.java
+++ b/src/main/java/gregtech/loaders/recipe/WoodMachineRecipes.java
@@ -94,7 +94,7 @@ public class WoodMachineRecipes {
                 }
             }
             //noinspection ConstantConditions
-            ModHandler.addShapedRecipe(outputRecipe.getRegistryName().getNamespace() + "_saw",
+            ModHandler.addShapedRecipe(outputRecipe.getRegistryName().getPath() + "_saw",
                 GTUtility.copyAmount(originalOutput, plankStack), "s", "L", 'L', stack);

             RecipeMaps.CUTTER_RECIPES.recipeBuilder().inputs(stack)

@warjort
Copy link
Copy Markdown
Contributor

warjort commented Mar 21, 2021

Just in case, the patches for (1) and (2) above are:

diff --git a/src/main/java/gregtech/api/util/LargeStackSizeItemStackHandler.java b/src/main/java/gregtech/api/util/LargeStackSizeItemStackHandler.java
index c2195ac2..ac5dd55c 100644
--- a/src/main/java/gregtech/api/util/LargeStackSizeItemStackHandler.java
+++ b/src/main/java/gregtech/api/util/LargeStackSizeItemStackHandler.java
@@ -39,7 +39,7 @@ public class LargeStackSizeItemStackHandler extends ItemStackHandler {
             tagCompound.setTag(BIG_STACK_SIZE_TAG_KEY, stackSizes);

             //fix size overflow of existing item tags
-            for (NBTBase itemBase : items.tagList) {
+            for (NBTBase itemBase : items) {
                 NBTTagCompound item = (NBTTagCompound) itemBase;

                 byte size = item.getByte(ITEM_COUNT_TAG_KEY);
diff --git a/src/main/java/gregtech/common/blocks/modelfactories/BakedModelHandler.java b/src/main/java/gregtech/common/blocks/modelfactories/BakedModelHandler.java
index 78545ea4..3a4ad77e 100644
--- a/src/main/java/gregtech/common/blocks/modelfactories/BakedModelHandler.java
+++ b/src/main/java/gregtech/common/blocks/modelfactories/BakedModelHandler.java
@@ -117,7 +117,7 @@ public class BakedModelHandler {

         @Override
         public ItemOverrideList getOverrides() {
-            return new ItemOverrideList();
+            return ItemOverrideList.NONE;
         }

         @Override

@serenibyss
Copy link
Copy Markdown
Collaborator Author

I implemented the changes suggested above, and was able to fully launch the game and load into my test worlds without issue. At this point, I believe this PR is "finished," though it will need some testing to make sure there is no other functionality change that could lead to bugs or crashes.

@serenibyss serenibyss marked this pull request as ready for review March 21, 2021 01:03
@LAGIdiot
Copy link
Copy Markdown
Member

Good work finding out what was wrong and getting it work. I will postpone this from next release as I would like to truly test it out so we can be sure that there are any bugs or side effects.

@LAGIdiot LAGIdiot added rsr: minor Release size requirements: Minor type: feature New feature or request labels Mar 21, 2021
@LAGIdiot LAGIdiot merged commit 8391466 into GregTechCE:master Apr 7, 2021
ALongStringOfNumbers pushed a commit to ALongStringOfNumbers/GregTech that referenced this pull request Apr 7, 2021
@serenibyss serenibyss deleted the mcpmapping branch June 7, 2022 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rsr: minor Release size requirements: Minor type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants