Fix forge blockstates and custom model loading (#6154)

* Reimplement forge blockstates variant through the use of a pseudo-model that handles the model loading, retexturing, custom data, etc. on behalf of the blockstates loader. This model gets injected into the model registry with an autogenerated unique name, to not collide with other model locations.
* Fix model loaders not being properly initialized by calling the reload method from the loader register function. In 1.12, registering a reload listener caused the listener to be called right away from the register method. This is not true anymore.

patches/minecraft/net/minecraft/client/renderer/model/BlockModelDefinition.java.patch

@@ -7,11 +7,11 @@
 +   @Deprecated
     public static BlockModelDefinition func_209577_a(BlockModelDefinition.ContainerHolder p_209577_0_, Reader p_209577_1_) {
 -      return JSONUtils.func_193839_a(p_209577_0_.field_209575_a, p_209577_1_, BlockModelDefinition.class);
-+      return fromJson(p_209577_0_, p_209577_1_, null);
++      return fromJson(p_209577_0_, p_209577_1_, null, null, null);
     }
 
-+   public static BlockModelDefinition fromJson(BlockModelDefinition.ContainerHolder containerHolderIn, Reader readerIn, @Nullable net.minecraft.util.ResourceLocation location) {
-+      return net.minecraftforge.client.model.BlockStateLoader.load(readerIn, location, containerHolderIn.field_209575_a);
++   public static BlockModelDefinition fromJson(BlockModelDefinition.ContainerHolder containerHolderIn, Reader readerIn, @Nullable net.minecraft.util.ResourceLocation location, @Nullable ModelBakery bakery, @Nullable java.util.function.BiConsumer<net.minecraft.util.ResourceLocation, IUnbakedModel> modelConsumer) {
++      return net.minecraftforge.client.model.BlockStateLoader.load(readerIn, location, containerHolderIn.field_209575_a, bakery, modelConsumer);
 +   }
 +
     public BlockModelDefinition(Map<String, VariantList> p_i46572_1_, Multipart p_i46572_2_) {

patches/minecraft/net/minecraft/client/renderer/model/ModelBakery.java.patch

@@ -8,8 +8,26 @@
 +   private final Map<Triple<ResourceLocation, net.minecraftforge.common.model.IModelState, Boolean>, IBakedModel> field_217850_G = Maps.newHashMap();
     private final Map<ResourceLocation, IUnbakedModel> field_217851_H = Maps.newHashMap();
     private final Map<ResourceLocation, IBakedModel> field_217852_I = Maps.newHashMap();
-    private final AtlasTexture.SheetData field_217853_J;
-@@ -142,12 +142,19 @@
+    private AtlasTexture.SheetData field_217853_J;
+@@ -107,9 +107,17 @@
+    });
+
+    public ModelBakery(IResourceManager p_i51735_1_, AtlasTexture p_i51735_2_, BlockColors p_i51735_3_, IProfiler p_i51735_4_) {
++      this(p_i51735_1_, p_i51735_2_, p_i51735_3_, true);
++      processLoading(p_i51735_4_);
++   }
++
++   protected ModelBakery(IResourceManager p_i51735_1_, AtlasTexture p_i51735_2_, BlockColors p_i51735_3_, boolean vanillaBakery) {
+       this.field_177598_f = p_i51735_1_;
+       this.field_177609_j = p_i51735_2_;
+       this.field_225365_D = p_i51735_3_;
++   }
++
++   protected void processLoading(IProfiler p_i51735_4_) {
+       p_i51735_4_.func_76320_a("missing_model");
+
+       try {
+@@ -142,12 +150,19 @@
 
        p_i51735_4_.func_219895_b("special");
        this.func_217843_a(new ModelResourceLocation("minecraft:trident_in_hand#inventory"));
@@ -29,16 +47,16 @@
        set.forEach((p_217833_0_) -> {
           field_177603_c.warn("Unable to resolve texture reference: {}", (Object)p_217833_0_);
        });
-@@ -288,7 +295,7 @@
+@@ -288,7 +303,7 @@
                       {
                          lvt_13_5_ = this.field_177598_f.func_199004_b(resourcelocation1).stream().map((p_217839_1_) -> {
                             try (InputStream inputstream = p_217839_1_.func_199027_b()) {
 -                              Pair<String, BlockModelDefinition> pair2 = Pair.of(p_217839_1_.func_199026_d(), BlockModelDefinition.func_209577_a(this.field_209610_F, new InputStreamReader(inputstream, StandardCharsets.UTF_8)));
-+                              Pair<String, BlockModelDefinition> pair2 = Pair.of(p_217839_1_.func_199026_d(), BlockModelDefinition.fromJson(this.field_209610_F, new InputStreamReader(inputstream, StandardCharsets.UTF_8), p_209598_1_));
++                              Pair<String, BlockModelDefinition> pair2 = Pair.of(p_217839_1_.func_199026_d(), BlockModelDefinition.fromJson(this.field_209610_F, new InputStreamReader(inputstream, StandardCharsets.UTF_8), p_209598_1_, this, this::func_209593_a));
                                return pair2;
                             } catch (Exception exception1) {
                                throw new ModelBakery.BlockStateDefinitionException(String.format("Exception loading blockstate definition: '%s' in resourcepack: '%s': %s", p_217839_1_.func_199029_a(), p_217839_1_.func_199026_d(), exception1.getMessage()));
-@@ -404,7 +411,12 @@
+@@ -404,7 +419,12 @@
 
     @Nullable
     public IBakedModel func_217845_a(ResourceLocation p_217845_1_, ISprite p_217845_2_) {
@@ -52,7 +70,7 @@
        if (this.field_217850_G.containsKey(triple)) {
           return this.field_217850_G.get(triple);
        } else {
-@@ -412,11 +424,11 @@
+@@ -412,11 +432,11 @@
           if (iunbakedmodel instanceof BlockModel) {
              BlockModel blockmodel = (BlockModel)iunbakedmodel;
              if (blockmodel.func_178310_f() == field_177606_o) {
@@ -66,7 +84,7 @@
           this.field_217850_G.put(triple, ibakedmodel);
           return ibakedmodel;
        }
-@@ -471,6 +483,10 @@
+@@ -471,6 +491,10 @@
        return this.field_225367_M;
     }
 

patches/minecraft/net/minecraft/client/renderer/model/Variant.java.patch

@@ -9,20 +9,11 @@
     private final ResourceLocation field_188050_a;
     private final ModelRotation field_188051_b;
     private final boolean field_188052_c;
-@@ -30,10 +30,16 @@
+@@ -30,6 +30,7 @@
        return this.field_188050_a;
     }
 
 +   @Deprecated
     public ModelRotation func_188048_b() {
        return this.field_188051_b;
     }
-
-+   @Override
-+   public net.minecraftforge.common.model.IModelState getState() {
-+      return this.field_188051_b;
-+   }
-+
-    public boolean func_188049_c() {
-       return this.field_188052_c;
-    }

src/main/java/net/minecraftforge/client/model/BlockStateLoader.java

@@ -24,17 +24,11 @@
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.nio.charset.StandardCharsets;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.Map.Entry;
-import java.util.Optional;
+import java.util.function.BiConsumer;
 
-import net.minecraft.client.renderer.model.IUnbakedModel;
-import net.minecraft.client.renderer.model.BlockModelDefinition;
-import net.minecraft.client.renderer.model.ModelRotation;
-import net.minecraft.client.renderer.model.Variant;
-import net.minecraft.client.renderer.model.VariantList;
+import net.minecraft.client.renderer.model.*;
 import net.minecraft.util.ResourceLocation;
 import net.minecraftforge.common.model.IModelState;
 import net.minecraftforge.common.model.TRSRTransformation;
@@ -61,7 +55,9 @@ public class BlockStateLoader
             .create();
 
     private static final Logger LOGGER = LogManager.getLogger();
-
+
+    private static long internalGeneratedModelId = 1;
+
     /**
      * Loads a BlockStates json file.
      * Will attempt to parse it as a Forge Enhanced version if possible.
@@ -75,7 +71,7 @@ public class BlockStateLoader
      *
      * @return Model definition including variants for all known combinations.
      */
-    public static BlockModelDefinition load(Reader reader, ResourceLocation location, final Gson vanillaGSON)
+    public static BlockModelDefinition load(Reader reader, ResourceLocation location, final Gson vanillaGSON, ModelBakery bakery, BiConsumer<ResourceLocation, IUnbakedModel> modelConsumer)
     {
         try
         {
@@ -98,10 +94,21 @@ public static BlockModelDefinition load(Reader reader, ResourceLocation location
                             boolean uvLock = var.getUvLock().orElse(false);
                             int weight = var.getWeight().orElse(1);
 
-                            if (var.isVanillaCompatible())
-                                mcVars.add(new Variant(var.getModel(), (ModelRotation)var.getState().orElse(ModelRotation.X0_Y0), uvLock, weight));
-                            else
-                                mcVars.add(new ForgeVariant(location, var.getModel(), var.getState().orElse(TRSRTransformation.identity()), uvLock, var.getSmooth(), var.getGui3d(), weight, var.getTextures(), var.getOnlyPartsVariant(), var.getCustomData()));
+                            ResourceLocation modelLocation = var.getModel();
+                            if (!var.isVanillaCompatible() && bakery != null)
+                            {
+                                modelLocation = new ResourceLocation(
+                                        "internal",
+                                        String.format("%d/%s/%s/%s", internalGeneratedModelId++, location.getNamespace(), location.getPath(),
+                                                entry.getKey().replace("=","_").replace(",","_"))
+                                );
+
+                                IUnbakedModel model = ForgeVariantHelper.prepareInjectedModel((ModelLoader)bakery, modelLocation, var.getModel(), var.getSmooth(), var.getGui3d(), var.getTextures(), var.getOnlyPartsVariant(), var.getCustomData());
+
+                                modelConsumer.accept(modelLocation, model);
+                            }
+
+                            mcVars.add(new ForgeVariant(modelLocation, var.getState().orElse(ModelRotation.X0_Y0), uvLock, weight));
                         }
                         variants.put(entry.getKey(), new VariantList(mcVars));
                     }
@@ -123,7 +130,7 @@ public static class Marker
         public int forge_marker = -1;
     }
 
-    //This is here specifically so that we do not have a hard reference to ForgeBlockStateV1.Variant in ForgeVariant
+    //This is here specifically so that we do not have a hard reference to ForgeBlockStateV1.Variant in this code
     public static class SubModel
     {
         private final IModelState state;
@@ -154,92 +161,84 @@ public SubModel(IModelState state, boolean uvLock, boolean smooth, boolean gui3d
         public ImmutableMap<String, String> getCustomData() { return customData; }
     }
 
-    private static class ForgeVariant extends Variant implements ISmartVariant
+    private static class ForgeVariant extends Variant
     {
-        private final ResourceLocation blockstateLocation;
-        private final ImmutableMap<String, String> textures;
-        private final ImmutableMap<String, SubModel> parts;
-        private final ImmutableMap<String, String> customData;
-        private final Optional<Boolean> smooth;
-        private final Optional<Boolean> gui3d;
         private final IModelState state;
 
-        ForgeVariant(ResourceLocation blockstateLocation, @Nullable ResourceLocation model, IModelState state, boolean uvLock, Optional<Boolean> smooth, Optional<Boolean> gui3d, int weight, ImmutableMap<String, String> textures, ImmutableMap<String, SubModel> parts, ImmutableMap<String, String> customData)
+        ForgeVariant(ResourceLocation model, IModelState state, boolean uvLock, int weight)
         {
-            super(model == null ? new ResourceLocation("builtin/missing") : model, state instanceof ModelRotation ? (ModelRotation)state : ModelRotation.X0_Y0, uvLock, weight);
-            this.blockstateLocation = blockstateLocation;
-            this.textures = textures;
-            this.parts = parts;
-            this.customData = customData;
+            super(model, state instanceof ModelRotation ? (ModelRotation)state : ModelRotation.X0_Y0, uvLock, weight);
             this.state = state;
-            this.smooth = smooth;
-            this.gui3d = gui3d;
         }
 
-        private IUnbakedModel runModelHooks(IUnbakedModel base, Optional<Boolean> smooth, Optional<Boolean> gui3d, ImmutableMap<String, String> textureMap, ImmutableMap<String, String> customData)
+        @Override
+        public IModelState getState()
         {
-            base = base.process(customData);
-            base = base.retexture(textureMap);
-            base = smooth.map(base::smoothLighting).orElse(base);
-            base = gui3d.map(base::gui3d).orElse(base);
-            return base;
+            return state;
         }
 
-        /**
-         * Used to replace the base model with a re-textured model containing sub-models.
-         */
         @Override
-        public IUnbakedModel process(IUnbakedModel base)
+        public String toString() {
+            return "Forge" + super.toString();
+        }
+    }
+
+    private static class ForgeVariantHelper
+    {
+        public static IUnbakedModel prepareInjectedModel(ModelLoader bakery, ResourceLocation blockstateLocation, @Nullable ResourceLocation modelLocation, Optional<Boolean> smooth, Optional<Boolean> gui3d, ImmutableMap<String, String> textures, ImmutableMap<String, SubModel> parts, ImmutableMap<String, String> customData)
         {
             int size = parts.size();
-            // FIXME: should missing base be handled this way?
-            boolean hasBase = base != ModelLoaderRegistry.getMissingModel();
 
-            if (hasBase)
+            IUnbakedModel base = null;
+            if (modelLocation != null)
             {
-                base = runModelHooks(base, smooth, gui3d, textures, customData);
+                try
+                {
+                    base = ModelLoaderRegistry.getModel(modelLocation);
+
+                    if (base != null)
+                    {
+                        base = base.process(customData);
+                        base = base.retexture(textures);
+                        base = smooth.map(base::smoothLighting).orElse(base);
+                        base = gui3d.map(base::gui3d).orElse(base);
 
-                if (size <= 0)
-                    return base;
+                        if (size <= 0)
+                            return base;
+                    }
+                }
+                catch (Exception e)
+                {
+                    LOGGER.error("Error processing base model for forge blockstates pseudo-model: '" + blockstateLocation, e);
+                }
             }
 
             ImmutableMap.Builder<String, Pair<IUnbakedModel, IModelState>> models = ImmutableMap.builder();
             for (Entry<String, SubModel> entry : parts.entrySet())
             {
                 SubModel part = entry.getValue();
 
-                final ResourceLocation modelLocation = part.getModelLocation();
+                final ResourceLocation location = part.getModelLocation();
                 final IUnbakedModel model;
-                if (modelLocation == null)
+                if (location == null)
                 {
                     LOGGER.error("model not found for variant {} for blockstate {}", entry.getKey(), blockstateLocation);
                     model = ModelLoaderRegistry.getMissingModel(blockstateLocation, new Throwable());
                 }
                 else
                 {
-                    model = ModelLoaderRegistry.getModelOrLogError(modelLocation, "Unable to load block sub-model: \'" + modelLocation);
+                    model = ModelLoaderRegistry.getModelOrLogError(location, "Unable to load block sub-model '" + entry.getKey() + "': '" + location + "'");
                 }
 
-                models.put(entry.getKey(), Pair.of(runModelHooks(model, Optional.of(part.smooth), Optional.of(part.gui3d), part.getTextures(), part.getCustomData()), part.getState()));
+                IUnbakedModel base1 = model;
+                base1 = base1.process(part.getCustomData());
+                base1 = base1.retexture(part.getTextures());
+                base1 = Optional.of(part.smooth).map(base1::smoothLighting).orElse(base1);
+                base1 = Optional.of(part.gui3d).map(base1::gui3d).orElse(base1);
+                models.put(entry.getKey(), Pair.of(base1, part.getState()));
             }
 
-            return new MultiModel(getModelLocation(), hasBase ? base : null, models.build());
-        }
-
-        @Override
-        public IModelState getState()
-        {
-            return state;
-        }
-
-        @Override
-        public String toString()
-        {
-            StringBuilder buf = new StringBuilder();
-            buf.append("TexturedVariant:");
-            for (Entry<String, String> e: this.textures.entrySet())
-                buf.append(" ").append(e.getKey()).append(" = ").append(e.getValue());
-            return buf.toString();
+            return new MultiModel(modelLocation, base, models.build());
         }
     }
 }

src/main/java/net/minecraftforge/client/model/ISmartVariant.java

@@ -21,6 +21,7 @@
 
 import net.minecraft.client.renderer.model.IUnbakedModel;
 
+@Deprecated
 public interface ISmartVariant
 {
     default IUnbakedModel process(IUnbakedModel base) {

src/main/java/net/minecraftforge/client/model/ModelLoader.java

@@ -136,10 +136,11 @@ public boolean isLoading()
 
     public ModelLoader(IResourceManager manager, AtlasTexture map, BlockColors colours, IProfiler profiler)
     {
-        super(manager, map, colours, profiler);
+        super(manager, map, colours, false);
         VanillaLoader.INSTANCE.setLoader(this);
         VariantLoader.INSTANCE.setLoader(this);
         ModelLoaderRegistry.clearModelCache(manager);
+        processLoading(profiler);
     }
 
     private static Set<ResourceLocation> specialModels = new HashSet<>();
@@ -483,8 +484,6 @@ public WeightedRandomModel(ResourceLocation parent, VariantList variants) throws
                     model = ModelLoaderRegistry.getModel(loc);
                 }
 
-                // FIXME: is this the place? messes up dependency and texture resolution
-                model = v.process(model);
                 for(ResourceLocation location : model.getDependencies())
                 {
                     ModelLoaderRegistry.getModelOrMissing(location);

src/main/java/net/minecraftforge/client/model/ModelLoaderRegistry.java

@@ -78,6 +78,9 @@ public static void registerLoader(ICustomModelLoader loader)
     {
         loaders.add(loader);
         ((IReloadableResourceManager) Minecraft.getInstance().getResourceManager()).addReloadListener(loader);
+        // FIXME: Existing model loaders expect to receive a call as soon as they are registered, which was the old behaviour pre-1.13
+        // without this, their manager field is never initialized.
+        loader.onResourceManagerReload(Minecraft.getInstance().getResourceManager());
     }
 
     public static boolean loaded(ResourceLocation location)

src/main/resources/META-INF/accesstransformer.cfg

@@ -44,6 +44,7 @@ public net.minecraft.client.renderer.model.ModelBakery field_177604_a # MODEL_MI
 protected net.minecraft.client.renderer.model.ModelBakery field_177606_o # MODEL_GENERATED
 protected net.minecraft.client.renderer.model.ModelBakery field_177609_j # textureMap
 protected net.minecraft.client.renderer.model.ModelBakery field_177616_r # MODEL_ENTITY
+private-f net.minecraft.client.renderer.model.ModelBakery field_217853_J # field_217853_J - need to un-finalize so that we can delay initialization to after calling super() in ModelLoader
 protected net.minecraft.client.renderer.model.ModelBakery func_177594_c(Lnet/minecraft/util/ResourceLocation;)Lnet/minecraft/client/renderer/model/BlockModel; # loadModel
 private-f net.minecraft.client.renderer.tileentity.PistonTileEntityRenderer field_178462_c # blockRenderer - it's static so we need to un-finalize in case this class loads to early.
 public net.minecraft.client.renderer.tileentity.TileEntityRendererDispatcher field_147557_n # fontRenderer - needed for rendering text in TESR items before entering world