`BakedModelManagerMixin.getModel` mixin overwrites Forge's equivalent patch

[SquidDev]

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:

https://github.com/Sinytra/ForgifiedFabricAPI/blob/641809b69d4f15991b9b090e828dff1c29d98a01/fabric-model-loading-api-v1/src/client/java/net/fabricmc/fabric/mixin/client/model/loading/BakedModelManagerMixin.java#L50-L53

However, this shadows a similar method within Forge:

https://github.com/MinecraftForge/MinecraftForge/blob/45b00b000411d9c7b2e090359b051dc60a73628a/patches/minecraft/net/minecraft/client/resources/model/ModelManager.java.patch#L23-L25

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 https://github.com/cc-tweaked/CC-Tweaked/issues/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 :(

[Su5eD]

Sorry about the confusion, it should be fixed now.

[SquidDev]

Thanks!