Allow non-block textures in JSON models (#438)

Co-authored-by: Martin Robertz <dream-master@gmx.net>
Co-authored-by: RecursivePineapple <recursive_pineapple@proton.me>

models.md

@@ -3,10 +3,13 @@
 In general, it is designed to mimic the latest version.
 
 ### Intentional deviations
-Texture references are assumed to be in `assets/<domain>/blocks` by
-default, to match 7.10 conventions. However, textures specified by
-`domain:block/whatever` are remapped to `domain:whatever`, to support
-importing models from modern.
+None (for now).
+
+### Former deviations
+Texture references are no longer assumed to be the same as 7.10 locations.
+Instead, they are resolved from `assets/namespace/textures` like modern
+does. This also means you can now use *any* texture in your ISBRH, and
+GTNHLib will inject it into the block atlas. Please use this power responsibly.
 
 Resource packs can now replace most blocks, similarly to modern.
 Note that this primarily works on blocks with ISBRHS - if the block
@@ -164,7 +167,6 @@ The loader will:
 Only implement the traits you need.
 
 ### TODO
-- Autoload item textures too.
 - Add smooth shading to ModelISBRH.
 - Add face culling to models.
 - Implement UV locking.

src/main/java/com/gtnewhorizon/gtnhlib/CommonProxy.java

@@ -35,6 +35,7 @@ import com.gtnewhorizon.gtnhlib.teams.TeamAdminCommand;
 import com.gtnewhorizon.gtnhlib.teams.TeamCommand;
 import com.gtnewhorizon.gtnhlib.test.block.BlockRngTest;
 import com.gtnewhorizon.gtnhlib.test.block.BlockTest;
+import com.gtnewhorizon.gtnhlib.test.block.BlockTestAo;
 import com.gtnewhorizon.gtnhlib.test.block.BlockTestLectern;
 import com.gtnewhorizon.gtnhlib.test.block.BlockTestTint;
 import com.gtnewhorizon.gtnhlib.test.block.BlockTestTintMul;
@@ -70,6 +71,7 @@ public class CommonProxy {
 
         if (GTNHLibConfig.enableTestBlocks) {
             BlockTest.register();
+            BlockTestAo.register();
             BlockTestLectern.register();
             BlockTestTint.register();
             BlockTestTintMul.register();

src/main/java/com/gtnewhorizon/gtnhlib/client/model/loading/ModelDeserializer.java

@@ -7,7 +7,6 @@ import static com.gtnewhorizon.gtnhlib.client.renderer.cel.model.quad.properties
 import java.lang.reflect.Type;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Pattern;
 
 import net.minecraftforge.common.util.ForgeDirection;
 
@@ -106,8 +105,6 @@ public class ModelDeserializer implements JsonDeserializer<JSONModel> {
         return ret;
     }
 
-    private static Pattern TEXEX = Pattern.compile("^([^:]+:)blocks?/");
-
     private static Object2ObjectOpenHashMap<String, String> loadTextures(JsonObject in) {
 
         final var textures = new Object2ObjectOpenHashMap<String, String>();
@@ -116,15 +113,6 @@ public class ModelDeserializer implements JsonDeserializer<JSONModel> {
             for (Map.Entry<String, JsonElement> e : in.getAsJsonObject("textures").entrySet()) {
                 String s = e.getValue().getAsString();
 
-                // If it's a texture variable, no munging is needed
-                if (!s.startsWith("#")) {
-                    // Add the default domain, if it's absent.
-                    if (!s.contains(":")) s = "minecraft:" + s;
-
-                    // Strip "block/" if present
-                    s = TEXEX.matcher(s).replaceFirst("$1");
-                }
-
                 // The key is always a variable, so prepend accordingly
                 final var key = e.getKey();
                 textures.put(key.startsWith("#") ? key : "#" + key, s);

src/main/java/com/gtnewhorizon/gtnhlib/client/model/loading/ModelRegistry.java

@@ -4,8 +4,6 @@ import static com.gtnewhorizon.gtnhlib.GTNHLibConfig.modelCacheSize;
 import static com.gtnewhorizon.gtnhlib.client.model.unbaked.MissingModel.MISSING_MODEL;
 import static com.gtnewhorizon.gtnhlib.core.GTNHLibCore.MODEL_LOGGER;
 
-import java.util.List;
-
 import net.minecraft.block.Block;
 import net.minecraft.client.resources.FallbackResourceManager;
 import net.minecraft.client.resources.IResourceManager;
@@ -38,8 +36,6 @@ import cpw.mods.fml.common.eventhandler.SubscribeEvent;
 import cpw.mods.fml.common.registry.GameData;
 import cpw.mods.fml.relauncher.Side;
 import cpw.mods.fml.relauncher.SideOnly;
-import it.unimi.dsi.fastutil.objects.ObjectArrayList;
-import it.unimi.dsi.fastutil.objects.ObjectLists;
 import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;
 
 /// Handles model loading and caching. All caches are size-based - this means that if a model has enough parents, it may
@@ -199,7 +195,7 @@ public class ModelRegistry {
 
             // Gather the list of modeled blocks and their textures
             final var modeledBlocks = new ObjectOpenHashSet<String>();
-            final var texturesToLoad = new ObjectArrayList<String>();
+            final var texturesToLoad = new ObjectOpenHashSet<String>();
             for (var pack : resourcePacks) {
                 if (!(pack instanceof ModelResourcePack mrp)) continue;
 
@@ -226,6 +222,8 @@ public class ModelRegistry {
                 texturesToLoad.addAll(info.textureNames());
             }
 
+            // Sadly, this field is untyped and we can't do anything about it... not easily, anyway.
+            // noinspection unchecked
             GameData.getBlockRegistry().registryObjects.forEach((s, b) -> {
                 if (!(s instanceof String name)) return;
                 if (!(b instanceof Block block)) return;
@@ -258,15 +256,15 @@ public class ModelRegistry {
 
     public static class EventHandler {
 
-        private static @NotNull List<String> texturesToLoad = ObjectLists.emptyList();
+        private static @NotNull ObjectOpenHashSet<String> texturesToLoad = ObjectOpenHashSet.of();
 
         @SubscribeEvent
         @SideOnly(Side.CLIENT)
         public void onTextureStitch(TextureStitchEvent.Pre event) {
             for (var texture : texturesToLoad) {
-                event.map.registerIcon(texture.replaceFirst("^minecraft:", ""));
+                TexHelper.registerTexture(event.map, texture);
             }
-            texturesToLoad = ObjectLists.emptyList(); // don't need it anymore
+            texturesToLoad = ObjectOpenHashSet.of(); // don't need it anymore
         }
     }
 }

src/main/java/com/gtnewhorizon/gtnhlib/client/model/loading/TexHelper.java

@@ -0,0 +1,30 @@
+package com.gtnewhorizon.gtnhlib.client.model.loading;
+
+import static com.gtnewhorizon.gtnhlib.core.GTNHLibCore.MODEL_LOGGER;
+
+import net.minecraft.client.renderer.texture.TextureMap;
+import net.minecraft.util.IIcon;
+
+/// This class exist to inject textures into one or more atlases, even if they were never intended to be there.
+public class TexHelper {
+
+    static IIcon registerTexture(TextureMap atlas, String texture) {
+        var icon = atlas.registerIcon(texture);
+        if (!(icon instanceof BaseCheckedTex bCT)) {
+            // Something has gone terribly wrong, and the map isn't returning a normal sprite
+            MODEL_LOGGER.warn("Failed to register {} in foreign atlas {}", texture, atlas);
+            return icon;
+        }
+
+        bCT.nhlib$setHasBase(false);
+        return icon;
+    }
+
+    /// Injected into [TextureAtlasSprite] so we can check if it has an implied base like `items` or `blocks`.`
+    public interface BaseCheckedTex {
+
+        void nhlib$setHasBase(boolean hasBase);
+
+        boolean nhlib$hasBase();
+    }
+}

src/main/java/com/gtnewhorizon/gtnhlib/client/model/unbaked/JSONModel.java

@@ -254,7 +254,6 @@ public class JSONModel implements UnbakedModel {
 
     // TODO Doesn't account for UV rotation settings atm
     protected void bakeSprite(ModelQuadViewMutable quad, String name) {
-        name = name.replaceFirst("^minecraft:", "");
         final var icon = Minecraft.getMinecraft().getTextureMapBlocks().getAtlasSprite(name);
         final float minU = icon.getMinU();
         final float minV = icon.getMinV();

src/main/java/com/gtnewhorizon/gtnhlib/mixins/Mixins.java

@@ -28,6 +28,8 @@ public enum Mixins implements IMixins {
             "models.MixinFileResourcePack",
             "models.MixinFolderResourcePack",
             "models.MixinRenderBlocks",
+            "models.MixinTextureAtlasSprite",
+            "models.MixinTextureMap",
             "models.SRRMAccessor").setPhase(Phase.EARLY).setApplyIf(() -> true)),
     MODEL_PARTICLE_FIXES(new MixinBuilder("Override particle icons for models.").addClientMixins(
             "models.particles.MixinEffectRenderer",

src/main/java/com/gtnewhorizon/gtnhlib/mixins/early/models/MixinTextureAtlasSprite.java

@@ -0,0 +1,25 @@
+package com.gtnewhorizon.gtnhlib.mixins.early.models;
+
+import net.minecraft.client.renderer.texture.TextureAtlasSprite;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Unique;
+
+import com.gtnewhorizon.gtnhlib.client.model.loading.TexHelper;
+
+@Mixin(TextureAtlasSprite.class)
+public class MixinTextureAtlasSprite implements TexHelper.BaseCheckedTex {
+
+    @Unique
+    private boolean nhlib$hasBase = true;
+
+    @Override
+    public void nhlib$setHasBase(boolean hasBase) {
+        nhlib$hasBase = hasBase;
+    }
+
+    @Override
+    public boolean nhlib$hasBase() {
+        return nhlib$hasBase;
+    }
+}

src/main/java/com/gtnewhorizon/gtnhlib/mixins/early/models/MixinTextureMap.java

@@ -0,0 +1,45 @@
+package com.gtnewhorizon.gtnhlib.mixins.early.models;
+
+import net.minecraft.client.renderer.texture.TextureAtlasSprite;
+import net.minecraft.client.renderer.texture.TextureMap;
+import net.minecraft.util.ResourceLocation;
+
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+
+import com.gtnewhorizon.gtnhlib.client.model.loading.TexHelper;
+import com.llamalad7.mixinextras.sugar.Local;
+
+@SuppressWarnings("UnusedMixin")
+@Mixin(TextureMap.class)
+public abstract class MixinTextureMap {
+
+    @Shadow
+    protected abstract ResourceLocation completeResourceLocation(ResourceLocation location, int mipId);
+
+    @Redirect(
+            method = "loadTextureAtlas",
+            at = @At(
+                    value = "INVOKE",
+                    target = "Lnet/minecraft/client/renderer/texture/TextureMap;completeResourceLocation(Lnet/minecraft/util/ResourceLocation;I)Lnet/minecraft/util/ResourceLocation;",
+                    ordinal = 0))
+    private ResourceLocation nhlib$unbaseTextures(TextureMap instance, ResourceLocation location, int mipId,
+            @Local(name = "textureatlassprite") TextureAtlasSprite sprite) {
+        if (((TexHelper.BaseCheckedTex) sprite).nhlib$hasBase()) return completeResourceLocation(location, mipId);
+
+        // I don't *like* completely replacing the internals, but it's better than an overwrite.
+        // And more importantly, I'm really not sure how I'd do this (cleanly) with a different method.
+        if (mipId == 0) {
+            return new ResourceLocation(
+                    location.getResourceDomain(),
+                    String.format("textures/%s%s", location.getResourcePath(), ".png"));
+        }
+
+        var path = location.getResourcePath().split("/", 2);
+        return new ResourceLocation(
+                location.getResourceDomain(),
+                String.format("textures/%s/mipmaps/%s.%d%s", path[0], path[1], mipId, ".png"));
+    }
+}

src/main/java/com/gtnewhorizon/gtnhlib/test/block/BlockTest.java

@@ -2,24 +2,12 @@ package com.gtnewhorizon.gtnhlib.test.block;
 
 import net.minecraft.block.Block;
 import net.minecraft.block.material.Material;
-import net.minecraft.entity.EntityLivingBase;
-import net.minecraft.item.ItemStack;
-import net.minecraft.world.World;
-import net.minecraftforge.common.util.ForgeDirection;
-
-import com.gtnewhorizon.gtnhlib.blockstate.properties.DirectionBlockProperty;
-import com.gtnewhorizon.gtnhlib.blockstate.properties.DirectionBlockProperty.AbstractDirectionBlockProperty;
-import com.gtnewhorizon.gtnhlib.blockstate.registry.BlockPropertyRegistry;
-import com.gtnewhorizon.gtnhlib.util.DirectionUtil;
 
 import cpw.mods.fml.common.registry.GameRegistry;
 
+/// A plain cube, nothing special about it.
 public class BlockTest extends Block {
 
-    /// This default blockstate
-    public final AbstractDirectionBlockProperty FACING_PROP = (AbstractDirectionBlockProperty) DirectionBlockProperty
-            .facing();
-
     public BlockTest() {
         super(Material.wood);
     }
@@ -28,25 +16,11 @@ public class BlockTest extends Block {
         final var testBlock = new BlockTest();
         testBlock.setBlockName("model_test");
         GameRegistry.registerBlock(testBlock, "model_test");
-
-        /// This method allows you to adapt any property to itemstacks, with some default value. This is mostly relevant
-        /// for modeled blocks, as they may require some state to match *any* of their models. If you're defining a
-        /// custom blockstate, it's better to add an ItemStack default yourself - see [BlockTestTintMul] for an example
-        /// of that.
-        BlockPropertyRegistry.registerBlockItemProperty(testBlock, testBlock.FACING_PROP, ForgeDirection.EAST);
     }
 
+    /// Okay so it's transparent, but that's it. That's the only special thing about it.
     @Override
     public boolean isOpaqueCube() {
         return false;
     }
-
-    @Override
-    public void onBlockPlacedBy(World worldIn, int x, int y, int z, EntityLivingBase placer, ItemStack itemIn) {
-        var dir = DirectionUtil.yawToDirection(placer.rotationYaw);
-
-        /// This is a shortcut that works because this block only has a single meta property. See [BlockTestTintMul] for
-        /// an example with non-meta blockstate.
-        worldIn.setBlockMetadataWithNotify(x, y, z, FACING_PROP.getMeta(dir, 0), 2);
-    }
 }

src/main/java/com/gtnewhorizon/gtnhlib/test/block/BlockTestAo.java

@@ -0,0 +1,52 @@
+package com.gtnewhorizon.gtnhlib.test.block;
+
+import net.minecraft.block.Block;
+import net.minecraft.block.material.Material;
+import net.minecraft.entity.EntityLivingBase;
+import net.minecraft.item.ItemStack;
+import net.minecraft.world.World;
+import net.minecraftforge.common.util.ForgeDirection;
+
+import com.gtnewhorizon.gtnhlib.blockstate.properties.DirectionBlockProperty;
+import com.gtnewhorizon.gtnhlib.blockstate.properties.DirectionBlockProperty.AbstractDirectionBlockProperty;
+import com.gtnewhorizon.gtnhlib.blockstate.registry.BlockPropertyRegistry;
+import com.gtnewhorizon.gtnhlib.util.DirectionUtil;
+
+import cpw.mods.fml.common.registry.GameRegistry;
+
+public class BlockTestAo extends Block {
+
+    /// This default blockstate
+    public final AbstractDirectionBlockProperty FACING_PROP = (AbstractDirectionBlockProperty) DirectionBlockProperty
+            .facing();
+
+    public BlockTestAo() {
+        super(Material.wood);
+    }
+
+    public static void register() {
+        final var testBlock = new BlockTestAo();
+        testBlock.setBlockName("model_test_ao");
+        GameRegistry.registerBlock(testBlock, "model_test_ao");
+
+        /// This method allows you to adapt any property to itemstacks, with some default value. This is mostly relevant
+        /// for modeled blocks, as they may require some state to match *any* of their models. If you're defining a
+        /// custom blockstate, it's better to add an ItemStack default yourself - see [BlockTestTintMul] for an example
+        /// of that.
+        BlockPropertyRegistry.registerBlockItemProperty(testBlock, testBlock.FACING_PROP, ForgeDirection.EAST);
+    }
+
+    @Override
+    public boolean isOpaqueCube() {
+        return false;
+    }
+
+    @Override
+    public void onBlockPlacedBy(World worldIn, int x, int y, int z, EntityLivingBase placer, ItemStack itemIn) {
+        var dir = DirectionUtil.yawToDirection(placer.rotationYaw);
+
+        /// This is a shortcut that works because this block only has a single meta property. See [BlockTestTintMul] for
+        /// an example with non-meta blockstate.
+        worldIn.setBlockMetadataWithNotify(x, y, z, FACING_PROP.getMeta(dir, 0), 2);
+    }
+}

src/main/java/com/gtnewhorizon/gtnhlib/test/block/BlockTestLectern.java

@@ -6,7 +6,7 @@ import com.gtnewhorizon.gtnhlib.blockstate.registry.BlockPropertyRegistry;
 
 import cpw.mods.fml.common.registry.GameRegistry;
 
-public class BlockTestLectern extends BlockTest {
+public class BlockTestLectern extends BlockTestAo {
 
     public BlockTestLectern() {
         super();

src/main/resources/assets/gtnhlib/blockstates/model_test.json

@@ -1,8 +1,5 @@
 {
   "variants": {
-    "facing=north": { "model": "gtnhlib:blocks/old_stairs", "y": 270, "uvlock": true },
-    "facing=south": { "model": "gtnhlib:blocks/old_stairs", "y": 90, "uvlock": true },
-    "facing=west": { "model": "gtnhlib:blocks/old_stairs", "y": 0, "uvlock": true },
-    "facing=east": { "model": "gtnhlib:blocks/old_stairs", "y": 180, "uvlock": true }
+    "": { "model": "gtnhlib:blocks/model_test" }
   }
 }

src/main/resources/assets/gtnhlib/blockstates/model_test_ao.json

@@ -0,0 +1,8 @@
+{
+  "variants": {
+    "facing=north": { "model": "gtnhlib:blocks/old_stairs", "y": 270, "uvlock": true },
+    "facing=south": { "model": "gtnhlib:blocks/old_stairs", "y": 90, "uvlock": true },
+    "facing=west": { "model": "gtnhlib:blocks/old_stairs", "y": 0, "uvlock": true },
+    "facing=east": { "model": "gtnhlib:blocks/old_stairs", "y": 180, "uvlock": true }
+  }
+}

src/main/resources/assets/gtnhlib/blockstates/model_test_lectern.json

@@ -1,8 +1,19 @@
 {
   "variants": {
-    "facing=north": { "model": "minecraft:block/lectern", "y": 270, "uvlock": true },
-    "facing=south": { "model": "minecraft:block/lectern", "y": 90, "uvlock": true },
-    "facing=west": { "model": "minecraft:block/lectern", "y": 0, "uvlock": true },
-    "facing=east": { "model": "minecraft:block/lectern", "y": 180, "uvlock": true }
+    "facing=east": {
+      "model": "minecraft:block/lectern",
+      "y": 90
+    },
+    "facing=north": {
+      "model": "minecraft:block/lectern"
+    },
+    "facing=south": {
+      "model": "minecraft:block/lectern",
+      "y": 180
+    },
+    "facing=west": {
+      "model": "minecraft:block/lectern",
+      "y": 270
+    }
   }
 }

src/main/resources/assets/gtnhlib/models/blocks/model_test.json

@@ -1,6 +1,12 @@
 {
-  "parent": "block/cube_all",
+  "parent": "block/cube",
   "textures": {
-    "all": "minecraft:stone"
+    "particle": "gtnhlib:blocks/test",
+    "north": "blocks/stone",
+    "south": "minecraft:blocks/oak_planks",
+    "east":  "items/blaze_rod",
+    "west":  "minecraft:items/apple",
+    "up":    "gtnhlib:blocks/test",
+    "down":  "gtnhlib:blocks/test"
   }
 }

src/main/resources/assets/gtnhlib/models/blocks/model_test_tint.json

@@ -1,10 +1,10 @@
 {
   "parent": "block/cube",
   "textures": {
-    "particle": "gtnhlib:block/test",
-    "north": "gtnhlib:block/test",
-    "south": "gtnhlib:block/test",
-    "east":  "gtnhlib:block/test",
+    "particle": "gtnhlib:blocks/test",
+    "north": "gtnhlib:blocks/test",
+    "south": "gtnhlib:blocks/test",
+    "east":  "gtnhlib:blocks/test",
     "west":  "gtnhlib:blocks/test",
     "up":    "gtnhlib:blocks/test",
     "down":  "gtnhlib:blocks/test"

src/main/resources/assets/gtnhlib/models/blocks/model_test_tint_mul.json

@@ -3,8 +3,8 @@
 	"credit": "Made with Blockbench",
 	"parent": "block/cube",
 	"textures": {
-		"particle": "planks_oak",
-		"down": "planks_oak"
+		"particle": "blocks/planks_oak",
+		"down": "blocks/planks_oak"
 	},
 	"elements": [
 		{
@@ -56,4 +56,4 @@
 			"scale": [0.5, 1.2, 1]
 		}
 	}
-}
+}

src/main/resources/assets/gtnhlib/models/blocks/old_stairs.json

@@ -1,8 +1,8 @@
 {
   "parent": "minecraft:block/stairs",
   "textures": {
-    "bottom": "minecraft:planks_oak",
-    "side": "minecraft:planks_oak",
-    "top": "minecraft:planks_oak"
+    "bottom": "minecraft:blocks/planks_oak",
+    "side": "minecraft:blocks/planks_oak",
+    "top": "minecraft:blocks/planks_oak"
   }
 }

src/main/resources/assets/gtnhlib/models/blocks/test_one.json

@@ -1,6 +1,6 @@
 {
   "parent": "block/cube_all",
   "textures": {
-    "all": "gtnhlib:test_one"
+    "all": "gtnhlib:blocks/test_one"
   }
 }

src/main/resources/assets/gtnhlib/models/blocks/test_three.json

@@ -1,6 +1,6 @@
 {
   "parent": "block/cube_all",
   "textures": {
-    "all": "gtnhlib:test_three"
+    "all": "gtnhlib:blocks/test_three"
   }
 }

src/main/resources/assets/gtnhlib/models/blocks/test_two.json

@@ -1,6 +1,6 @@
 {
   "parent": "block/cube_all",
   "textures": {
-    "all": "gtnhlib:test_two"
+    "all": "gtnhlib:blocks/test_two"
   }
 }

src/main/resources/assets/minecraft/models/block/lectern.json

@@ -13,12 +13,12 @@
     }
   },
   "textures": {
-    "particle": "minecraft:lectern_sides",
-    "bottom": "minecraft:oak_planks",
-    "base": "minecraft:lectern_base",
-    "front": "minecraft:lectern_front",
-    "sides": "minecraft:lectern_sides",
-    "top": "minecraft:lectern_top"
+    "particle": "minecraft:blocks/lectern_sides",
+    "bottom": "minecraft:blocks/oak_planks",
+    "base": "minecraft:blocks/lectern_base",
+    "front": "minecraft:blocks/lectern_front",
+    "sides": "minecraft:blocks/lectern_sides",
+    "top": "minecraft:blocks/lectern_top"
   },
   "elements": [
     {