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

Run sync tasks scheduled for the same tick FIFO #114

Closed
wants to merge 1 commit into from

Conversation

fantahund
Copy link

@fantahund fantahund commented Sep 26, 2022

@EverNife
Copy link
Member

I started to look for this patch.

Are you sure that the "task.getID()" ensure the order?


https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java#83

I think on the latestes versions of spigot they store the nanoSecond of creation of the task and compare based on that.

@Brokkonaut
Copy link

It was changed here: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/4f34a67b5ed70f88ef857d13de1ee3ec433f0ad7 but thats more or less another issue - currently with or without the changes in pr #113 the server would fail when taskids overflow, because negative and possible duplicate taskids are not expected in several places.

@juanmuscaria
Copy link
Member

Looks good, I'll be merging it to staging once I finish reworking the release cycle.
It may be important to also bring that other spigot patch as well.

@juanmuscaria
Copy link
Member

Could you retarget this pull request to the staging branch?

@fantahund fantahund changed the base branch from master to staging September 29, 2022 20:05
@EverNife
Copy link
Member

It was changed here: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/4f34a67b5ed70f88ef857d13de1ee3ec433f0ad7 but thats more or less another issue - currently with or without the changes in pr #113 the server would fail when taskids overflow, because negative and possible duplicate taskids are not expected in several places.

I think its better to implement the new system

@juanmuscaria juanmuscaria self-assigned this Feb 6, 2024
@juanmuscaria
Copy link
Member

Manually added at 31a50c6

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