-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat: Armor Stand and armor model support #242
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
yes, sorry I didn't think much about it. yes, there should be a lot of code cleanup around entities. especially if I'm going to also use alternative renderers In the future. I was going to clean & rewrite entity code a long time ago, but still didn't find an ideal solution yet. Of course, any improvements are welcome. Imo it all should be split into classes with clear interfaces so we clearly understand dependency of the code and it can be more modular... |
) | ||
} | ||
|
||
const armorStandMeta = getSpecificEntityMetadata('armor_stand', entity) | ||
if (armorStandMeta) { |
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.
i think we have to split this huge update function into other functions like updateThing at some point
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.
Yeah, I have been thinking about how to better handle this too. Having a section for each entity would really bloat it, maybe it even needs to be in a separate file for each entity to properly support all of the functionality.
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.
Oh yes thats a good idea!
const hasBasePlate = (parseInt(armorStandMeta.client_flags, 10) & 0x08) === 0 | ||
const isMarker = (parseInt(armorStandMeta.client_flags, 10) & 0x10) !== 0 | ||
mesh.castShadow = !isMarker | ||
mesh.receiveShadow = !isMarker |
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.
i like it! 👏
As always, Lgtm thanks a lot for your awesome contributions! Should it be merged or further work is planned? |
This part is as finished as I can get it for now before the Holidays and as I wrote the armor stand and basic armor rendering works so yeah, it's meant to be mergedable. |
This PR adds support for armor stand options (small, arms, baseplate, posing) as well as rendering of armor on armor stands (as well as other entities and players but they aren't animated properly there yet, players unfortunately use a different library so this work would need to be replicated there. (which is questionable, imo. everything entity-related should use a single system) For now one just sees that the armor is worn but not the animations. On armor stands they are posed according to the armor stand pose) so improvements to this could be to properly animate them in the future as well as load the models from resource packs. (Which right now have no concept of armor textures and I couldn't really figure out a good approach so something for later)
In order to do that the missing
mirror
option of the model system was also implemented in EntityMeshSome tests: