-
-
Notifications
You must be signed in to change notification settings - Fork 77
Map most block state model classes 1.21.8 #739
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
base: 1.21.8
Are you sure you want to change the base?
Conversation
…del package; finish mapping a couple more things
….model.json is for plain models themselves), fix some things
supersaiyansubtlety
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.
This is good coverage, thanks!
The breakdown and explanations were super helpful, and I especially like the ModelVariantMap -> BlockstateDefinition refactor; nice catch of an outdated name!
I did a bit of a deep dive review, so I have some nit picks and a couple suggestions, but overall this is a great PR.
| METHOD m_pwxgnxdm withOrientation (Lnet/minecraft/unmapped/C_blazqqmb$C_ufdansrr;)Lnet/minecraft/unmapped/C_blazqqmb; | ||
| ARG 1 orientation | ||
| METHOD m_yfzzsmuo orientation ()Lnet/minecraft/unmapped/C_blazqqmb$C_ufdansrr; | ||
| CLASS C_ufdansrr Orientation |
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.
| CLASS C_ufdansrr Orientation | |
| CLASS C_ufdansrr ModelState |
uvLock doesn't really fit in Orientation
| CLASS net/minecraft/unmapped/C_blazqqmb net/minecraft/client/render/block/model/ModelVariant | ||
| FIELD f_qhvszzcj orientation Lnet/minecraft/unmapped/C_blazqqmb$C_ufdansrr; | ||
| FIELD f_szkyblfb location Lnet/minecraft/unmapped/C_ncpywfca; | ||
| FIELD f_zldxgrxu CODEC Lcom/mojang/serialization/MapCodec; |
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.
| FIELD f_zldxgrxu CODEC Lcom/mojang/serialization/MapCodec; | |
| FIELD f_zldxgrxu MAP_CODEC Lcom/mojang/serialization/MapCodec; |
to differentiate it from the non-map codec field
| @@ -0,0 +1,34 @@ | |||
| CLASS net/minecraft/unmapped/C_blazqqmb net/minecraft/client/render/block/model/ModelVariant | |||
| FIELD f_qhvszzcj orientation Lnet/minecraft/unmapped/C_blazqqmb$C_ufdansrr; | |||
| FIELD f_szkyblfb location Lnet/minecraft/unmapped/C_ncpywfca; | |||
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.
| FIELD f_szkyblfb location Lnet/minecraft/unmapped/C_ncpywfca; | |
| FIELD f_szkyblfb id Lnet/minecraft/unmapped/C_ncpywfca; |
qmap tends to use "id"/"identifier" over "location"; qmap's Identifier is mojmap's ResourceLocation
| METHOD m_jwusibwq withIdentifier (Lnet/minecraft/unmapped/C_ncpywfca;)Lnet/minecraft/unmapped/C_blazqqmb; | ||
| ARG 1 identifier |
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.
| METHOD m_jwusibwq withIdentifier (Lnet/minecraft/unmapped/C_ncpywfca;)Lnet/minecraft/unmapped/C_blazqqmb; | |
| ARG 1 identifier | |
| METHOD m_jwusibwq withId (Lnet/minecraft/unmapped/C_ncpywfca;)Lnet/minecraft/unmapped/C_blazqqmb; |
to match the field; the param gets automatically mapped to id
| ARG 1 yQuadrant | ||
| METHOD m_hcdcljgq withUvLock (Z)Lnet/minecraft/unmapped/C_blazqqmb; | ||
| ARG 1 uvLock | ||
| METHOD m_ihmyzhvs location ()Lnet/minecraft/unmapped/C_ncpywfca; |
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.
| METHOD m_ihmyzhvs location ()Lnet/minecraft/unmapped/C_ncpywfca; | |
| METHOD m_ihmyzhvs id ()Lnet/minecraft/unmapped/C_ncpywfca; |
| METHOD m_grlikkhm collectBlockModels (Lnet/minecraft/unmapped/C_rlomrsco;Ljava/util/List;)V | ||
| ARG 1 random | ||
| ARG 2 bakedModels |
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.
| METHOD m_grlikkhm collectBlockModels (Lnet/minecraft/unmapped/C_rlomrsco;Ljava/util/List;)V | |
| ARG 1 random | |
| ARG 2 bakedModels | |
| METHOD m_grlikkhm collectParts (Lnet/minecraft/unmapped/C_rlomrsco;Ljava/util/List;)V | |
| ARG 1 random | |
| ARG 2 parts |
to match the updated name for C_uudyvsyc (currently BakedBlockModel)
| METHOD m_grlikkhm collectBlockModels (Lnet/minecraft/unmapped/C_rlomrsco;Ljava/util/List;)V | ||
| ARG 1 random | ||
| ARG 2 bakedModels | ||
| METHOD m_ikisitoo getBlockModels (Lnet/minecraft/unmapped/C_rlomrsco;)Ljava/util/List; |
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.
| METHOD m_ikisitoo getBlockModels (Lnet/minecraft/unmapped/C_rlomrsco;)Ljava/util/List; | |
| METHOD m_ikisitoo chooseParts (Lnet/minecraft/unmapped/C_rlomrsco;)Ljava/util/List; |
to match the updated name for C_uudyvsyc (currently BakedBlockModel)
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.
this isn't really a "model" itself, so I think something like WeightedVariants would be more accurate
| METHOD m_ytjbermp loadPartsForStates (Lnet/minecraft/unmapped/C_ezfeikaq;)Lnet/minecraft/unmapped/C_teykdqpg$C_wyawmrcu; | ||
| ARG 1 stateManager | ||
| CLASS C_qsaqchhw Variants | ||
| FIELD f_gjkxbbmc parts Ljava/util/Map; |
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.
Part for C_iqnucppo is a little off here
the "parts" don't combine to create one model; one complete model is selected
C_iqnucppo is Unbaked on mojmap and yarn, which I think makes more sense here; this field would be models
though this change would require renaming some other inner classes of BlockStateModel
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.
Since this class contains only static members, conventionally the name would either be plural or have a Util suffix to make it clearer that it's not meant for instantiation.
This PR adds mappings for most classes related to loading and baking the models of blocks from their block state definition files. The biggest (breaking?) change in this PR is renaming
n.m.client.render.model.ModelVariantMaptoBlockstateDefinition, the rationale for which is given below.Because there's a lot of new stuff, here's a rough guide to how it relates to the block state definition JSON files:
And the classes that the definitions turn into before baking:
n.m.client.render.block.BlockStateModel(interface): a block state modelWeightedBlockStateModel: a block state model with multiple (weighted) parts to be chosen to be renderedSingleBlockStateModel: a block state model with one part to be renderedMultipartBlockStateModel: a block state model comprised of multiple models to be rendered, depending on if their selectors match the block stateAnd after baking:
n.m.client.render.block.BakedBlockModel(interface): a baked block model, with methodsgetQuadsandgetParticleSpriteC_eqeibyds, i don't know enough about Minecraft's rendering to map this.Other classes (most in
n.m.client.render.model.):BlockStateLoader: the main class that manages loading block state definitions and turning them into unbaked models for bakingItemModelLoader: the same thing, but for item models. not mapped other than the name, a couple fields, and theload()methodBakedModelManager: just renamed RESOURCE_NAMESPACE to MODEL_NAMESPACE (there's multiple resource namespaces used, this one is for"models")ModelBaker: just an interface forModelLoader.ModelLoaderBakern.m.data.client.model.ModelVariantModifier: utility class for modifyingModelVariants during data generationn.m.client.render.model.WeightedUnbakedModelton.m.data.client.model.WeightedUnbakedModel; it appears to only be used during data generationfor the only breaking change: i renamed ModelVariantMap to BlockstateDefinition because
n.m.client.render.item.model.BaseItemModel)assets/<namespace>/blockstates/<entry>.jsoni tried to follow the naming conventions & guidelines as much as i could, but this is my first mappings PR; let me know if anything should be changed :)