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

[Slayer Helpers] Slayer Highlights, Blaze Helpers and Glow Caching #888

Merged
merged 9 commits into from
Aug 12, 2024

Conversation

BigloBot
Copy link
Contributor

@BigloBot BigloBot commented Aug 3, 2024

Remade this PR cause The merge conflicts were horrible

  • Hitbox Slayer Mob Highlighting
  • Glow Effect Slayer Mob Highlighting
  • Blazeslayer Attunement highlighting
  • Fire Pillar Countdown Notifiications

This PR Also reworks "ShouldGlow" Detection

  • There's a "truesight" aura of 20 blocks - I think this is fine because this is just about in range of where nametags begin to shrink to the point of being undreadable, and thus could be reasonably argued you could tell this is where a mob is.
  • Anything beyond this distance is raycast to ensure it can be seen
  • caches "shouldGlow" for 50ms/1 ticks duration
  • caches "canSee" for 100ms/2 ticks duration

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Aug 3, 2024
@BigloBot BigloBot changed the title [Slayer Helpers] remade slayer helpers pr [Slayer Helpers] Slayer Highlights, Blaze Helpers and Glow Caching Aug 3, 2024
@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label Aug 4, 2024
@AzureAaron AzureAaron added this to the 1.22 milestone Aug 4, 2024
@AzureAaron AzureAaron mentioned this pull request Aug 4, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Aug 5, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Aug 7, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Aug 10, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

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

Btw should be the last review. I have also referenced Aaron's previous review.

@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Aug 10, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Aug 12, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels Aug 12, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Aug 12, 2024
Copy link
Collaborator

@AzureAaron AzureAaron left a comment

Choose a reason for hiding this comment

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

Looks good, in future we will need to optimize the glow/box calculations to not constantly run when an entity is rendered.

@BigloBot
Copy link
Contributor Author

BigloBot commented Aug 12, 2024

Looks good, in future we will need to optimize the glow/box calculations to not constantly run when an entity is rendered.

I'm sure there's a fair bit more optimisation that can be done, but I noticed that i go from ~0.7% doing these calcs while idling on my island -> 0.25% with these changes which was more than enough for me to be happy with lol

@AzureAaron
Copy link
Collaborator

Looks good, in future we will need to optimize the glow/box calculations to not constantly run when an entity is rendered.

I'm sure there's a fair bit more optimisation that can be done, but I noticed that i go from ~0.7% doing these calcs while idling on my island -> 0.25% with these changes which was more than enough for me to be happy with lol

I was talking about iterating the scoreboard to check if we're in a slayer or not. My idea is roughly to create an event for when the mod updates the scoreboard, check the updated scoreboard value and set a static boolean that can be read from. I wasn't talking about anything beyond that though

@BigloBot
Copy link
Contributor Author

BigloBot commented Aug 12, 2024

Looks good, in future we will need to optimize the glow/box calculations to not constantly run when an entity is rendered.

I'm sure there's a fair bit more optimisation that can be done, but I noticed that i go from ~0.7% doing these calcs while idling on my island -> 0.25% with these changes which was more than enough for me to be happy with lol

I was talking about iterating the scoreboard to check if we're in a slayer or not. My idea is roughly to create an event for when the mod updates the scoreboard, check the updated scoreboard value and set a static boolean that can be read from. I wasn't talking about anything beyond that though

ahh, well technically that's getting cached too so it's not every frame render, but ur suggestion definitely a far better implementation than the current mechanism

@AzureAaron AzureAaron merged commit 3830cea into SkyblockerMod:master Aug 12, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This issue or PR is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants