`BakedModelManagerMixin.getModel` mixin overwrites Forge's equivalent patch
SquidDev opened this issue ยท 2 comments
Describe the bug
Tested with:
- Minecraft 1.20.1
- Forge 47.2.20
- Forgified Fabric API 0.91.0+1.10.4+1.20.1
- Connector 1.0.0-beta.32+1.20.1
Fabric API (and thus Forgified Fabric API) adds a method to ModelManager/BakedModelManagerMixin to get the model from a ResourceLocation/Identifier:
However, this shadows a similar method within Forge:
While this would be fine, the two methods have different implementations - the Forge method is guaranteed to not return null, while the Fabric one does. This means that any mod which expects this method to not return null, will crash if the model cannot be loaded.
This has been seen in cc-tweaked/CC-Tweaked#1626, where (for unrelated reasons) a model could not be loaded, which caused a crash when trying to use that model.
Logs
No response
Additional context
Just to confirm that which method implementation was present in the class, I enabled mixin dumping in an environment with just Forgified Fabric API installed:
$ javap -c net.minecraft.client.resources.model.ModelManager | grep --after=10 getModel\(
public net.minecraft.client.resources.model.BakedModel getModel(net.minecraft.resources.ResourceLocation);
Code:
0: aload_0
1: getfield #92 // Field f_119397_:Ljava/util/Map;
4: aload_1
5: invokeinterface #421, 2 // InterfaceMethod java/util/Map.get:(Ljava/lang/Object;)Ljava/lang/Object;
10: checkcast #126 // class net/minecraft/client/resources/model/BakedModel
13: areturn
I wonder if it's worth having something as part of the build process which ensures that this does not happen again. Unfortunately I don't believe any of mixin's debug options will catch this :(