-
Notifications
You must be signed in to change notification settings - Fork 487
Armor Renderer Upgrades #4916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Armor Renderer Upgrades #4916
Conversation
...ic-rendering-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/FabricModel.java
Outdated
Show resolved
Hide resolved
...-v1/src/client/java/net/fabricmc/fabric/impl/client/rendering/ArmorRendererRegistryImpl.java
Outdated
Show resolved
Hide resolved
fabric-rendering-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/ModelMixin.java
Outdated
Show resolved
Hide resolved
...-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/EntityRenderManagerMixin.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Juuz <[email protected]>
Co-authored-by: Juuz <[email protected]>
Co-authored-by: Juuz <[email protected]>
|
I wouldn't say it's entirely impossible. For example, I'm making use of a custom render command for my in-progress porting of Mine little Pony to 1.21.10 |
modmuss50
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks great, just a few questions.
...ng-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TransformCopyingModel.java
Outdated
Show resolved
Hide resolved
...ng-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/TransformCopyingModel.java
Outdated
Show resolved
Hide resolved
...-rendering-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/ArmorRenderer.java
Outdated
Show resolved
Hide resolved
modmuss50
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but really needs someone familar with this area to approve.
Juuxel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks generally fine to me.
I have a custom ArmorRenderer in 1.21.10 that doesn't need these changes, but I assume that's because I render block models instead of entity models.
...-rendering-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/ArmorRenderer.java
Outdated
Show resolved
Hide resolved
...-rendering-v1/src/client/java/net/fabricmc/fabric/api/client/rendering/v1/ArmorRenderer.java
Outdated
Show resolved
Hide resolved
* Add EquipmentModelData to EntityModelLayerRegistry * Add TransformCopyingModel * Upgrade ArmorRenderer * Update tests * Mark FabricModel as NonExtendable * Fix FabricModel doc Co-authored-by: Juuz <[email protected]> * Make ModelMixin package-private Co-authored-by: Juuz <[email protected]> * Make EntityRenderManagerMixin package-private Co-authored-by: Juuz <[email protected]> * Remove try-catch block in ArmorRendererRegistryImpl * Make TransformCopyingModel final * Use spaces for alignment in ArmorRenderer --------- Co-authored-by: Juuz <[email protected]> (cherry picked from commit bee81f0)
Vanilla's rendering overhaul with the
RenderCommandQueuehas made it nearly impossible to create a custom armor renderer that can render properly on any biped entity with theArmorRendererin its current implementation. In Vanilla, thesetAnglesmethod for models is now called when aModelCommandis rendered, meaning that calling it before submitting a model to the render queue is not a reliable way to transform a model.For example, the current armor rendering test mod adds a renderer for a helmet and chestplate. The model has visibility of its parts set based on the equipment slot, but because this happens before the model is submitted, it leads to visibility conflicts when the helmet and chestplate are worn at the same time. Furthermore, the model's arms swing when it is worn on an armor stand (ignores the armor stand's poses too) and it does not scale properly on differently sized biped mobs.
I am creating this pull request to not only provide a solution for this issue, but to also provide some related upgrades that help with armor rendering.
This PR includes:
ArmorRenderer.ArmorRendererneed a mutable and nullable model field that is checked on render and instantiated ifnull.TransformCopyingModelModel<? super S>) and delegate (Model<? super D>) model field, with its render state holding both the source and delegate states at the same time (Pair<S, D>). When thesetAnglesmethod for this model is called, it sets the source model's angles using the first pair value, copies the transforms to the delegate model, then optionally sets the angles for the delegate model. The delegate model is what gets rendered.TransformCopyingModelhave been added toArmorRenderer, based on therenderPartstatic method that used to be there.EquipmentModelData<EntityModelLayer>EquipmentModelDataclass, this allows users to do the same.FabricModelinjected interfacegetChildPartandcopyTransformsmethods, which were needed to make theTransformCopyingModelto function. I find them useful enough to users for this to be a injected interface rather than just a mixin duck.