From 6a91982acc1ff441bdf31da3ce27f50db49fc7a2 Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 27 Aug 2023 12:54:05 +0200 Subject: [PATCH 1/2] Fix Random mismatch between vanilla and FRAPI baked models (weighted and multipart) --- .../client/MultipartBakedModelMixin.java | 16 +- .../client/WeightedBakedModelMixin.java | 12 +- .../src/testmod/resources/fabric.mod.json | 1 + .../renderer/client/RandomSupplierTest.java | 138 ++++++++++++++++++ 4 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 fabric-renderer-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/renderer/client/RandomSupplierTest.java diff --git a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java index 8b68ff8530..5a5808ff23 100644 --- a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java +++ b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java @@ -81,15 +81,23 @@ public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos Pair, BakedModel> pair = components.get(i); if (pair.getLeft().test(state)) { - pair.getRight().emitBlockQuads(blockView, state, pos, randomSupplier, context); bitSet.set(i); } } stateCache.put(state, bitSet); - } else { - for (int i = 0; i < this.components.size(); i++) { - if (bitSet.get(i)) components.get(i).getRight().emitBlockQuads(blockView, state, pos, randomSupplier, context); + } + + Random random = randomSupplier.get(); + long randomSeed = random.nextLong(); + Supplier subModelSupplier = () -> { + random.setSeed(randomSeed); + return random; + }; + + for (int i = 0; i < this.components.size(); i++) { + if (bitSet.get(i)) { + components.get(i).getRight().emitBlockQuads(blockView, state, pos, subModelSupplier, context); } } } diff --git a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java index 9729a9b817..d2e4f86996 100644 --- a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java +++ b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java @@ -71,7 +71,11 @@ public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos Weighted.Present selected = Weighting.getAt(this.models, Math.abs((int) randomSupplier.get().nextLong()) % this.totalWeight).orElse(null); if (selected != null) { - selected.getData().emitBlockQuads(blockView, state, pos, randomSupplier, context); + selected.getData().emitBlockQuads(blockView, state, pos, () -> { + Random random = randomSupplier.get(); + random.nextLong(); // Imitate vanilla going through the weighted list every time + return random; + }, context); } } @@ -80,7 +84,11 @@ public void emitItemQuads(ItemStack stack, Supplier randomSupplier, Rend Weighted.Present selected = Weighting.getAt(this.models, Math.abs((int) randomSupplier.get().nextLong()) % this.totalWeight).orElse(null); if (selected != null) { - selected.getData().emitItemQuads(stack, randomSupplier, context); + selected.getData().emitItemQuads(stack, () -> { + Random random = randomSupplier.get(); + random.nextLong(); // Imitate vanilla going through the weighted list every time + return random; + }, context); } } } diff --git a/fabric-renderer-api-v1/src/testmod/resources/fabric.mod.json b/fabric-renderer-api-v1/src/testmod/resources/fabric.mod.json index 95124366a1..36a5bc49c8 100644 --- a/fabric-renderer-api-v1/src/testmod/resources/fabric.mod.json +++ b/fabric-renderer-api-v1/src/testmod/resources/fabric.mod.json @@ -15,6 +15,7 @@ "net.fabricmc.fabric.test.renderer.RendererTest" ], "client": [ + "net.fabricmc.fabric.test.renderer.client.RandomSupplierTest", "net.fabricmc.fabric.test.renderer.client.RendererClientTest" ] } diff --git a/fabric-renderer-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/renderer/client/RandomSupplierTest.java b/fabric-renderer-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/renderer/client/RandomSupplierTest.java new file mode 100644 index 0000000000..d79782a621 --- /dev/null +++ b/fabric-renderer-api-v1/src/testmodClient/java/net/fabricmc/fabric/test/renderer/client/RandomSupplierTest.java @@ -0,0 +1,138 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.test.renderer.client; + +import java.util.List; +import java.util.function.Supplier; + +import org.apache.commons.lang3.tuple.Pair; +import org.jetbrains.annotations.Nullable; + +import net.minecraft.block.BlockState; +import net.minecraft.block.Blocks; +import net.minecraft.client.render.model.BakedModel; +import net.minecraft.client.render.model.BakedQuad; +import net.minecraft.client.render.model.MultipartBakedModel; +import net.minecraft.client.render.model.WeightedBakedModel; +import net.minecraft.client.render.model.json.ModelOverrideList; +import net.minecraft.client.render.model.json.ModelTransformation; +import net.minecraft.client.texture.Sprite; +import net.minecraft.util.collection.Weighted; +import net.minecraft.util.math.BlockPos; +import net.minecraft.util.math.Direction; +import net.minecraft.util.math.random.Random; +import net.minecraft.world.BlockRenderView; + +import net.fabricmc.api.ClientModInitializer; +import net.fabricmc.fabric.api.renderer.v1.render.RenderContext; + +/** + * Tests that vanilla and Fabric API give the same random results. + * + *

Never do this in a real mod, this is purely for testing! + */ +public class RandomSupplierTest implements ClientModInitializer { + private static long previousRandom = 0; + private static boolean hasPreviousRandom = false; + + @Override + public void onInitializeClient() { + var checkingModel = new RandomCheckingBakedModel(); + var weighted = new WeightedBakedModel(List.of( + Weighted.of(checkingModel, 1), + Weighted.of(checkingModel, 2))); + var multipart = new MultipartBakedModel(List.of( + Pair.of(state -> true, weighted), + Pair.of(state -> true, weighted))); + var weightedAgain = new WeightedBakedModel(List.of( + Weighted.of(multipart, 1), + Weighted.of(multipart, 2))); + + long startingSeed = 42; + Random random = Random.create(); + + random.setSeed(startingSeed); + weightedAgain.getQuads(Blocks.STONE.getDefaultState(), null, random); + + random.setSeed(startingSeed); + weightedAgain.getQuads(Blocks.STONE.getDefaultState(), null, random); + + Supplier randomSupplier = () -> { + random.setSeed(startingSeed); + return random; + }; + weightedAgain.emitBlockQuads(null, Blocks.STONE.getDefaultState(), BlockPos.ORIGIN, randomSupplier, null); + } + + private static class RandomCheckingBakedModel implements BakedModel { + @Override + public List getQuads(@Nullable BlockState state, @Nullable Direction face, Random random) { + long value = random.nextLong(); + + if (hasPreviousRandom) { + if (value != previousRandom) { + throw new AssertionError("Random value is not the same as the previous one!"); + } + } else { + hasPreviousRandom = true; + previousRandom = value; + } + + return List.of(); + } + + @Override + public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos pos, Supplier randomSupplier, RenderContext context) { + getQuads(state, null, randomSupplier.get()); + } + + @Override + public boolean useAmbientOcclusion() { + return false; + } + + @Override + public boolean hasDepth() { + return false; + } + + @Override + public boolean isSideLit() { + return false; + } + + @Override + public boolean isBuiltin() { + return false; + } + + @Override + public Sprite getParticleSprite() { + return null; + } + + @Override + public ModelTransformation getTransformation() { + return null; + } + + @Override + public ModelOverrideList getOverrides() { + return null; + } + } +} From 07787311fc570652a327ed4aca98245260ab5b2c Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Mon, 4 Sep 2023 10:49:45 +0200 Subject: [PATCH 2/2] Review comments --- .../mixin/renderer/client/MultipartBakedModelMixin.java | 5 +++-- .../mixin/renderer/client/WeightedBakedModelMixin.java | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java index 5a5808ff23..ce8aca7350 100644 --- a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java +++ b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/MultipartBakedModelMixin.java @@ -89,15 +89,16 @@ public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos } Random random = randomSupplier.get(); + // Imitate vanilla passing a new random to the submodels long randomSeed = random.nextLong(); - Supplier subModelSupplier = () -> { + Supplier subModelRandomSupplier = () -> { random.setSeed(randomSeed); return random; }; for (int i = 0; i < this.components.size(); i++) { if (bitSet.get(i)) { - components.get(i).getRight().emitBlockQuads(blockView, state, pos, subModelSupplier, context); + components.get(i).getRight().emitBlockQuads(blockView, state, pos, subModelRandomSupplier, context); } } } diff --git a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java index d2e4f86996..53c508121a 100644 --- a/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java +++ b/fabric-renderer-api-v1/src/client/java/net/fabricmc/fabric/mixin/renderer/client/WeightedBakedModelMixin.java @@ -73,7 +73,7 @@ public void emitBlockQuads(BlockRenderView blockView, BlockState state, BlockPos if (selected != null) { selected.getData().emitBlockQuads(blockView, state, pos, () -> { Random random = randomSupplier.get(); - random.nextLong(); // Imitate vanilla going through the weighted list every time + random.nextLong(); // Imitate vanilla modifying the random before passing it to the submodel return random; }, context); } @@ -86,7 +86,7 @@ public void emitItemQuads(ItemStack stack, Supplier randomSupplier, Rend if (selected != null) { selected.getData().emitItemQuads(stack, () -> { Random random = randomSupplier.get(); - random.nextLong(); // Imitate vanilla going through the weighted list every time + random.nextLong(); // Imitate vanilla modifying the random before passing it to the submodel return random; }, context); }