Skip to content

Rewrite framed map tracker ticking #9605

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

Closed

Conversation

Warriorrrr
Copy link
Member

@Warriorrrr Warriorrrr commented Aug 14, 2023

Rewrites the tracking code for framed maps to remove the use of the tickCarriedBy method and the HoldingPlayer class. the tickCarriedBy method contained a lot of code that did not apply to framed maps at all, and by moving the parts that did elsewhere, we can essentially skip it.

Closes #9597

@Warriorrrr Warriorrrr force-pushed the feat/frame-tracker-rewrite branch from c76e3c6 to ea10f5d Compare September 6, 2023 06:09
@Warriorrrr Warriorrrr marked this pull request as ready for review September 6, 2023 06:09
@Warriorrrr Warriorrrr requested a review from a team as a code owner September 6, 2023 06:09
@Warriorrrr Warriorrrr force-pushed the feat/frame-tracker-rewrite branch from ea10f5d to bd7c062 Compare September 6, 2023 14:10
@MrPowerGamerBR
Copy link
Contributor

MrPowerGamerBR commented Nov 27, 2023

I've been running this patch on my server for ~1 week+ (after making my own makeshift patch that did only the ItemFrame getItem() optimization, only to find out that someone else already had made the same optimization but better) and I couldn't find any issues, and it also improves performance a bit if the server has a lot of item frames (which is my case for a Survival server) by avoiding calling getItem(). Before the patch, all getItem()calls were using 0.25ms every tick. (not a lot but for big servers every optimization is a big W)

The only thing is that this patch needs to be rebased (again) for the latest version for it to compile, because MapDecoration's getType() is now type(): SparklyPower/SparklyPaper@27b4f21

@MrPowerGamerBR
Copy link
Contributor

Outfit8TSB

This comment was marked as spam.

@Warriorrrr
Copy link
Member Author

I was looking over this PR again and it seems like my approach here was a bit flawed, since it did not account for one framed map being viewed in multiple locations and being updated, only players viewing the item frame that's first in the entity iteration order would've seen the changes.

There are still some other gains that could be made, such as reducing the amount of map update packets being sent to players while no changes actually occurred, and the mess that is the tickCarriedBy (seriously, maps in item frames shouldn't have to check if the player is wearing a pumpkin).

@Warriorrrr Warriorrrr closed this Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Optimize MapItemSavedData#tickCarriedBy
4 participants