-
Notifications
You must be signed in to change notification settings - Fork 67
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
Halloween holiday feature #269
base: fabric-1.21.4
Are you sure you want to change the base?
Conversation
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.
Just a few TODO-ish notes
|
I will look what i can do with my limited coding knowledge. (You should have access to contribute in this pr) |
You don't have to make any of these changes, they're more just
That checkbox only applies to collaborators with push access to the upstream repository |
I know that is why i did it by the fork. |
I might have a texture for a mask I can use from the old Advanced Skin Customization mod. |
If I'm honest, I kinda disagree. I feel like no one would notice it or even try to use it if it's disabled by default. It's easy for people to go into the mod and disable it if they don't want it enabled, and it's only for 3 days of the entire year (currently). I've said before I have different plans for this feature eventually (i.e. able to select certain basic cosmetics to equip in a separate menu), but as of right now I'm leaving it as is. If there's a way to disable it for NPC players by default, I might do that. |
Note that I only explicitly mentioned I'd also be open to simply making this default value a separate (enabled by default) global config option though, but I'm generally just not a fan of having these apply to all player entities currently, with no way to remove them on NPCs.
While there isn't a universal method to determine if a player entity is an NPC, you could generally get a pretty good idea by checking |
bc3ab5a
to
00d3ecd
Compare
# Conflicts: # src/main/java/com/wildfire/main/WildfireHelper.java # src/main/java/com/wildfire/render/GenderLayer.java
Works again now |
|
||
halloweenMask.render(matrixStack, vertexConsumer, light, overlay); | ||
} catch(Exception e) { | ||
WildfireGender.LOGGER.error("Failed to render breast layer", e); |
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.
These should probably have different messages, although if vanilla rendering code throws an error we probably have bigger issues
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.
These probably just shouldn't be wrapped in try/catch blocks entirely, tbh.
Thx celeste
What would be better the current time span or only the thirty first? |
Wrong button, didn't mean to request a review oops... |
Some please make a good halloween mask or something