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

add nametag/hit brightness fix #433

Merged

Conversation

kurrycat2004
Copy link
Contributor

  • fixes the nametag of spiders, endermen and ender dragons being rendered too dark, because the lightmap coords were getting overwritten and not reset
  • fixes spiders, endermen and ender dragons being rendered too red when hit, because the blend function was overwritten and not reset

@wlhlm
Copy link
Member

wlhlm commented Nov 7, 2024

Thank you for submitting the PR. I was table to manually test this and can confirm this fixes what it says on the tin. However, I have very little knowledge of rendering so I have to defer on others to judge the implementation.

Before:
2024-11-07_12 05 38

After:
2024-11-07_13 09 02

@wlhlm
Copy link
Member

wlhlm commented Nov 7, 2024

What I'm wondering though, none of the mixins you are adding are specific to the mobs you've mentioned above. Why don't other entities have that problem? Does it really make sense to apply this fix to all entities being rendered, or could we fix the render for the entities mentioned?

@kurrycat2004
Copy link
Contributor Author

this problem only occurs because vanilla changes the gl state in shouldRenderPass but never resets it. only endermen, spiders and ender dragons have the specific problem i fixed, because they are the only mobs that render an extra translucent layer for the entity eyes, and therefore setup the blend func and lightmap tex coords for that. both fixes also reset the state to the state it would normally be in for all other mobs, so it wont break anything. i implemented it for every mob in case some mod looks at vanilla code and implements entity eyes the same way.

@wlhlm
Copy link
Member

wlhlm commented Nov 7, 2024

Thanks for the explanation. That makes sense. 👍 I'll defer to @mitchej123 for final approval.

@wlhlm wlhlm requested review from mitchej123 and a team November 7, 2024 19:29
@mitchej123 mitchej123 merged commit 4aaf07d into GTNewHorizons:master Nov 7, 2024
1 check passed
@mitchej123
Copy link
Contributor

Thanks for the tag, forgot about this.

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

Successfully merging this pull request may close these issues.

3 participants