[1.16.5] Add Entity Visibility API#5993
Conversation
JRoy
left a comment
JRoy
left a comment
There was a problem hiding this comment.
didn't look at server changes yet, just a few API things
| + * @param entity the entity to check | ||
| + * @return True if this player can see the given entity | ||
| + */ | ||
| + boolean canSee(@NotNull org.bukkit.entity.Entity entity); |
There was a problem hiding this comment.
This will require explicit casts for people who want Player#canSee(Player). Should call this method canSeeEntity to avoid this.
There was a problem hiding this comment.
And no, this does not require explicit casts to call the method with the Player argument.
| + * Resets to which players this entity is shown and updates the default shown or hidden state | ||
| + * @param shownByDefault True if entity should not be visible to players by default | ||
| + */ | ||
| + void resetAndSetShownByDefault(boolean shownByDefault); |
There was a problem hiding this comment.
holy fuck I hate this name, maybe don't bundle these operations?
There was a problem hiding this comment.
The implementation only saves the players per entity which differ from the default state.
This combination saves us from iterating through all players to toggle their existence in the impl WeakHashSet (potentially twice).
I'd prefer to just change the name to something less ugly.
|
Currently testing. |
|
Must've accidentally forgotten to test after the latest changes I did |
|
I'm not sure if new API is going to be merged into 1.16.5. I see you've created a PR for 1.17 which is good. Once a new version is out, usually only bug fixes are backported, and even those only for a short while until the new version has been out for a couple months. |
|
bump |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
1.16.5 is EOL. |
4 participants
Closes #5973
This PR adds Entity Visibility API by injecting itself into the tracker logic roughly similar to how things were done for showPlayer/hidePlayer by upstream.
When the new methods are called with a Player as argument, they redirect to the existing showPlayer/hidePlayer/canSee methods.
Some sound effects are not using Entity#playSound method.I decided that changing that it out of scope for this PR/patch, a followup patch / PR can easily change that, playSound and sendPacketNearby methods are prepared.Now in this PR/patch as well
Feedback appreciated
Edit:
Once I tested this, I'll open another PR to also add the feature to 1.17PR for 1.17: #6000