Skip to content
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

[1.21.4] Fix IClientItemExtensions#renderHelmetOverlay never being called #1837

Open
wants to merge 3 commits into
base: 1.21.x
Choose a base branch
from

Conversation

IThundxr
Copy link
Contributor

No description provided.

@neoforged-automation neoforged-automation bot added the 1.21.4 Targeted at Minecraft 1.21.4 label Jan 10, 2025
@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@neoforged-compatibility-checks
Copy link

neoforged-compatibility-checks bot commented Jan 11, 2025

@IThundxr, this PR introduces breaking changes.
Fortunately, this project is currently accepting breaking changes, but if they are not intentional, please revert them.
Last checked commit: a0e84abf218f3a513d4c5843ba4dd46411107090.

neoforge (:neoforge)

  • net/neoforged/neoforge/client/extensions/common/IClientItemExtensions
    • renderHelmetOverlay(Lnet/minecraft/world/item/ItemStack;Lnet/minecraft/world/entity/player/Player;IIF)V: ❗ API method was removed

@ChampionAsh5357
Copy link
Contributor

So, this method needs a rename and some smarter handling imo. Basically, what Gui#renderCamerOverlays is doing is checking every equipment slot to see whether the equipped item (with sanity checks for is actually equippable and in the proper slot) has a camera overlay. The else statement just says, 'oh attempt to render this every time regardless whether the item is in the correct location'. For example, even holding an item with the method implemented (instead of wearing it on the head) would call the rendering code.

Technically there are a few other asssumptions being made, such as the player must be in first person for the effect to be applied, but we'll assume it's only going to be rendered for first-person cameras that are not scoping. So, for this case, it would make more sense to call the method something like renderFirstPersonOverlay and have it render in conjunction with the texture overlay that can be present.

@IThundxr
Copy link
Contributor Author

IThundxr commented Jan 11, 2025

So, this method needs a rename and some smarter handling imo. Basically, what Gui#renderCamerOverlays is doing is checking every equipment slot to see whether the equipped item (with sanity checks for is actually equippable and in the proper slot) has a camera overlay. The else statement just says, 'oh attempt to render this every time regardless whether the item is in the correct location'. For example, even holding an item with the method implemented (instead of wearing it on the head) would call the rendering code.

Technically there are a few other asssumptions being made, such as the player must be in first person for the effect to be applied, but we'll assume it's only going to be rendered for first-person cameras that are not scoping. So, for this case, it would make more sense to call the method something like renderFirstPersonOverlay and have it render in conjunction with the texture overlay that can be present.

Those would be some good changes, i'll get them implemented and backported to 1.21.1

@IThundxr
Copy link
Contributor Author

@ChampionAsh5357 I've went ahead and made some changes, let me know if you want any other changes

+ this.renderTextureOverlay(p_316735_, equippable.cameraOverlay().get().withPath(p_380782_ -> "textures/" + p_380782_ + ".png"), 1.0F);
+ }
+
+ if (equipmentslot == EquipmentSlot.HEAD) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it matter whether it's on the head or not? The point is to allow it for any equipment slot if it matches where its supposed to be equipped to.

+ this.renderTextureOverlay(p_316735_, equippable.cameraOverlay().get().withPath(p_380782_ -> "textures/" + p_380782_ + ".png"), 1.0F);
+ }
+
+ if (equipmentslot == EquipmentSlot.HEAD) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it matter whether it's on the head or not? The point is to allow it for any equipment slot if it matches where its supposed to be equipped to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.4 Targeted at Minecraft 1.21.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants