Extend BukkitScheduler with new/simplified methods#11825
Conversation
|
My concern with exposing stuff like Duration is that it sounds like the API is making a level of promise that it can actually fulfil a wall clock/"human" based delay when it blatantly cannot |
|
How does this interact with this command https://minecraft.wiki/w/Commands/tick? How does it deal with the low TPS drift? Something along the lines of https://www.creativedeletion.com/2015/01/28/falsehoods-programmers-date-time-zones.html - the long standing problem is, as electronicboy said, the wall time scheduling that has plagued the game forever. If I want to schedule a restart at exactly 18:00 the next day - what do I do? It's obvious the server cannot be at exactly 20.00 TPS (or any 1.00x TPS speed) and will always undershoot the desired frequency. In a favourable scenario, the scheduled restart task will occur within 1 minute, but be greatly off with fluctuations in actual TPS. Another example, suppose you have birthday today. Your next birthday isn't 365 days from now, it is exactly one calendar year from now. Otherwise you'd be surprised if your next birthday was offset by a day thanks to a February 29th. Let's assume the function interfaces are fine: does Paper want to publish an inaccurate implementation? Won't the users assume and adopt to the "seconds are roughly ticks" implementation and be wrong if the impl changes to more accurately schedule wall time events? If a plugin developer writes this seconds->ticks calculation algorithm, the pitfalls should be clear to them. If this is part of a core API, this understanding is not a given. Why not provide these separately as a util class to make this separation obvious? As an end end-user (not just client code) I would love to finally have date-time aligned events/scheduling. However I think the scheduler would need to be accurate and resilient enough, like a cron daemon implementation. |
|
scheduleSyncDelayedTask etc is legacy stuff, we all use runtasklater/runtasktimer now |
|
I generally also do not seem much use for this due to the above reasons. |
|
Yeah the durationj thing is misleading if its 50ms res |
|
Given no one else picked up on this, I'll be closing this as I do not think we'll be merging it. |
|
Sorry for the late reply, maybe someone will still look here, so I thought that it could be worth mentioning what motivated me to create this PR, or rather what was I basing it on. It's hard for me to disagree with the above answers, and I won't discuss, but there's a little inconsistency in official stance on this topic. In the documentation of scheduler, a similar method of converting units to the one I proposed in this PR is shown. There is a small mention in upper section that if the server runs 10 ticks per second, the conversion rate to human time will be different, but if it is such a problem as you say, in my opinion it should be more emphasized, especially since below, there is an entire section with a bolded header dedicated to the topic of 'ignorant' (referring to the above discussion, this is not intended to offend anyone who created this part of the documentation) unit conversion. Anyway, I understand why my PR was rejected and I'm writing this just so the rest of users won't be unaware for a similar reason |
For quite some time, I’ve noticed various plugins implementing strange calculations for converting real time into ticks, especially when scheduling tasks at real-world dates and times. These implementations often involve a lot of complexity around time zones.
In this PR, I’ve added several methods aimed at simplifying scheduler usage. I utilized Java’s modern time API, including
Duration,LocalDateTimeandLocalTimeAdded API:
DurationDurationintervalLocalDateTime. Time zone handling and potential clock adjustments are managed automatically without user interventionLocalTime. As with the above, time zone handling does not require user intervention. This required the creation of a new class extendingCraftTask. In the current version of this PR, I implemented this only for synchronous tasks, as doing it for asynchronous ones would be more complex. I’d like to hear your opinion on this approach first. The existing class has been coded in such a way to share most of the code if the async version was createdThis was quite challenging to implement, cause the
CraftSchedulercodebase is very inconsistent. I tried to model my methods on the existing ones in style and structure. I did not touch methods that were annotated as deprecated. I believe the scheduler’s overall design is something that should be addressed, but I refrained from making changes myself as I’m unsure if that’s something the team desires.I’ve provided proper Javadocs for all the new methods, based on the existing ones and adding some additional notes for users to consider.
I’m not entirely sure how the team prefers these methods to be implemented or which parts of the API should be designed differently. However, I think that these methods will help simplify plugin code and improve its readability. If implemented and documented correctly, these methods shouldn’t cause any issues.
I will be grateful for your feedback and happy to adapt to it