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

The Crouch Mystery #127

Merged

Conversation

iHDeveloper
Copy link
Contributor

@iHDeveloper iHDeveloper commented Aug 26, 2022

A mystery that awaits us

It took about a week to figure out and solve the issue, probably @InventivetalentDev would have resolved it faster and quicker, but it was fun to solve for me. :)

Description

It appears that when the player sneaks/crouches in 1.19+. The glow around the player vanishes sadly.

Cause

After doing a deep investigation into NMS. I reached to this analysis provided in the picture below.

glow-api-analysis
Analysis of outgoing packets from the server to the target (player)

It appears that when the player sneaks (crouches) a metadata packet is sent with the sneaking flag enabled (and without the glow flag enabled as expected). What's interesting here is that the sneaking flag is stored in the ENTITY_SHARED_FLAGS data watcher object. (are we expecting many entities being able to sneak? I don't really know. It will be fun to see though. lol)

This metadata packet is sent from an update players function that gets triggered from PlayerChunkMap tick method. The entity tracker gets called (by the update players function) and sends to the player the metadata packet of its new state.

Screenshots

player-standing-with-glow

  • Player standing but glowing as well as other entities around him

player-sneaking-with-glow

  • Player sneaking but glowing as well as other entities around him

Solution

We inject a packet handler using PacketListenerAPI to catch any metadata packet with the interested data watcher object (ENTITY_SHARED_FLAGS).

Then, we compare the glow data between the player who receives the packet and the entity thats the target of the glow flag in the metadata packet. We modify the packet when one of the parties are registered in the GlowAPI. If not, we just ignore the packet and proceed to listen to other packets.

Note: I added a map to register the entity unique id (UUID) by the entity id. It helps to avoid the async access to Bukkit/CraftBukkit APIs. And, it provides easy access in general. When the entity id is not found we just ignore modifying the packet in general.

Additional Fixes

  • [92ca605] Looking for libraries without the craftbukkit prefix (only for 1.19+)
  • [2fb5d58] Extending the class lookup to be used for 1.18+ versions.

Notes

  • Is the issue reproducible on Minecraft 1.18.x?

References

In 1.19.2, the libraries are loaded without the prefix
of the class path which is `org.bukkit.craftbukkit.libs` \o/
It appears in `1.19+`, A metadata packet is sent whenever the player
is sneaking (crouching) or un-sneaking (standing).

This commit implements an intercepter using **PacketListenerAPI**.
It watches for the metadata packet and intercept the glow value inside them
(if required). This intercepter is injected for 1.19+ only.
It appears that the libraries prefix `org.bukkit.craftbukkit.libs` is gone
from 1.18+ versions as well. Interesting.
@TherealB9B
Copy link

Also does this fix the glow vanishing with sprinting? Iirc it goes away while sprinting too and idk anything about github stuff so is this fix already in any jar file that works on 1.19.2?

@iHDeveloper
Copy link
Contributor Author

Also does this fix the glow vanishing with sprinting? Iirc it goes away while sprinting too and idk anything about github stuff so is this fix already in any jar file that works on 1.19.2?

Picture the PR as installing a body guard that make sure that every party involved with GlowAPI has the right glow information. This body guard will ignore anything unrelated to GlowAPI. So, anything like spiriting or sneaking will not matter as much as long as this body guard is making sure the right glow information is given to the right players/entities.

Basically, it will also fix the sprinting issue. :)

@TherealB9B
Copy link

Also does this fix the glow vanishing with sprinting? Iirc it goes away while sprinting too and idk anything about github stuff so is this fix already in any jar file that works on 1.19.2?

Picture the PR as installing a body guard that make sure that every party involved with GlowAPI has the right glow information. This body guard will ignore anything unrelated to GlowAPI. So, anything like spiriting or sneaking will not matter as much as long as this body guard is making sure the right glow information is given to the right players/entities.

Basically, it will also fix the sprinting issue. :)

Great! But wdym with the first part?

@iHDeveloper
Copy link
Contributor Author

Basically it makes sure that the correct glow information provided by the GlowAPI is already sent to the player

@TherealB9B
Copy link

Basically it makes sure that the correct glow information provided by the GlowAPI is already sent to the player

Ah okay. So will the fix be in the next update/jar of glowapi?

@iHDeveloper
Copy link
Contributor Author

That I leave to Inventivetalent. She can review it whenever she wants. After all, she is the expert. No need for rushing :)

@TherealB9B
Copy link

That I leave to Inventivetalent. She can review it whenever she wants. After all, she is the expert. No need for rushing :)

Yeah I understand. Sorry if it sounded like I was rushing/pushing anyone btw. I was just curious haha

@iHDeveloper
Copy link
Contributor Author

It's okay. I was just answering your question. If you have any about the PR, you can ask them here.

@iHDeveloper
Copy link
Contributor Author

This PR will also solve the new issued issue #128 due to the solution being at the packet level (Supposing the entity will not show unless with a packet that shows it and a metadata that describes it).

Note: This is purely speculative analysis. I didn't give it a test. But, I will soon :)

@srnyx
Copy link

srnyx commented Sep 14, 2022

This PR will also solve the new issued issue #128 due to the solution being at the packet level (Supposing the entity will not show unless with a packet that shows it and a metadata that describes it).

Note: This is purely speculative analysis. I didn't give it a test. But, I will soon :)

Yep, looks to fixed, thank you! @InventivetalentDev

@srnyx
Copy link

srnyx commented Sep 15, 2022

Although... I think it broke color codes somehow (it only started doing this when I started using this GlowAPI PR):
image

Copy link
Owner

@InventivetalentDev InventivetalentDev left a comment

Choose a reason for hiding this comment

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

really nice PR, thank you! :)
I made a few more changes & merged your packet handling with the existing handler, the mystery seems to be solved though!

@InventivetalentDev
Copy link
Owner

@srnyx I don't see anything that would break color codes specifically, maybe there's something else interfering with it?
I'll also go ahead and redirect your thanks to @iHDeveloper, he put all the work into this! ^^

@InventivetalentDev InventivetalentDev merged commit 8340c25 into InventivetalentDev:master Sep 15, 2022
@joeyblog joeyblog mentioned this pull request Aug 1, 2023
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.

4 participants