Spongeapi: Replace integer ticks durations with Duration across the API.

Created on 23 Apr 2018  路  4Comments  路  Source: SpongePowered/SpongeAPI

Originally @Meronat's idea

What do you think about introducing methods which accept a Duration instead of the long/double/int representing ticks in the API. Works better for other implementations than vanilla(e.g. lantern), easier to understand for plugin developers, and can just deprecate the old methods.
I've noticed issues where plugin developers misinterpret the parameter and it will be better for our implementation anyway.

All the usages of ticks would be replaced by Duration, or other time API related objects (which fit the usecase). Ticks is widely used across the API, this change will cause a lot of breaking changes.

Thoughts?

bleeding input wanted

Most helpful comment

I think we need to do this, as an addition, as I got caught out by it. I mentioned at the time that a Duration version may be a good idea, though it slipped my mind as I was at work at the time so forgot to bring it up later here. I'm glad someone else remembered to do so.

It is important to note throught documentation that supplying a Duration doesn't guarantee that the effect or whatever _will be_ that duration, which might be due to the tick system or some other effect. We need to be clear in the Javadocs that the duration supplied will be converted to the internal representation and may differ slightly due to server performance. Whether that is by a lower ticks per second or that the duration specifies a period does not divide evenly into ticks (though in the vast majority of cases, it will) doesn't matter really.

In defence of the current function - having a tick based function indicates that you want something to run for x ticks. Ultimately, specifying 20 ticks is 20 ticks, which is an undefined temporal duration but is guaranteed to be 20 ticks in the absence of any overriding calls. How long a tick is is at the mercy of the impl, though Sponge indicates that the server aims for 20 TPS, really, only the MCP implementations aim for that.

However, there is an elephant in the room that is worth mentioning. There is a wider problem here when it comes to purity of the API that's worth considering, though I don't think there is much that we can or should do about it. It's worth pointing out that server TPS is technically impl dependent. A plugin assuming 20 TPS (there is no API for target TPS) will work with the MCP versions, but another impl might run at 10TPS or 40TPS, or not use ticks at all. In fact, I'd view using ticks as a time measurement, or really, ticks _existing_, is an implementation leak in the wider scheme of things. Some other implementation might not use a tick system, it might do something else...

All 4 comments

For where we accept integers, we can deprecate those more easily, but return types are a bit more challenging and will cause breaking changes.

Switching to Duration provides less confusion for developers as they often misinterpret the parameter or return type, even with proper javadocs. It is also a better choice as not all implementations will necessarily limit the game to 20 tps.

I think we need to do this, as an addition, as I got caught out by it. I mentioned at the time that a Duration version may be a good idea, though it slipped my mind as I was at work at the time so forgot to bring it up later here. I'm glad someone else remembered to do so.

It is important to note throught documentation that supplying a Duration doesn't guarantee that the effect or whatever _will be_ that duration, which might be due to the tick system or some other effect. We need to be clear in the Javadocs that the duration supplied will be converted to the internal representation and may differ slightly due to server performance. Whether that is by a lower ticks per second or that the duration specifies a period does not divide evenly into ticks (though in the vast majority of cases, it will) doesn't matter really.

In defence of the current function - having a tick based function indicates that you want something to run for x ticks. Ultimately, specifying 20 ticks is 20 ticks, which is an undefined temporal duration but is guaranteed to be 20 ticks in the absence of any overriding calls. How long a tick is is at the mercy of the impl, though Sponge indicates that the server aims for 20 TPS, really, only the MCP implementations aim for that.

However, there is an elephant in the room that is worth mentioning. There is a wider problem here when it comes to purity of the API that's worth considering, though I don't think there is much that we can or should do about it. It's worth pointing out that server TPS is technically impl dependent. A plugin assuming 20 TPS (there is no API for target TPS) will work with the MCP versions, but another impl might run at 10TPS or 40TPS, or not use ticks at all. In fact, I'd view using ticks as a time measurement, or really, ticks _existing_, is an implementation leak in the wider scheme of things. Some other implementation might not use a tick system, it might do something else...

Whilst I agree, I think ticks are a relatively fundamental game design concept, let alone something fundamental to Mojang derived Minecraft servers, so it wouldn't be strictly terrible to leak.

Generally when someone schedules something on a tick basis, as opposed to time they are either using the wrong abstraction, or wish to do something periodically in game for performance reasons, as opposed to 'every tick'

If it's the type signature that's confusing, maybe we could address that with alternate names, or exposing a type for ticks, although, it's worth noting that https://docs.oracle.com/javase/8/docs/api/java/time/temporal/TemporalUnit.html#isTimeBased--

Supports isTimeBased, isDateBased, and isDurationEstimated so it might be possible to define an implementation dependent temporal unit known as a tick.

Things that arn't tick dependent can possibly make assumptions and convert it to seconds.

However, I'm very much against this solution vs methods with naming with explicit reference to ticks.

As it complicates the API, and would need people to know Ticks exist in the API, rather then just using auto complete to use Java's defaults, that would likely be recommended by most IDE's over a class provided by us with TermporalUnit constants.

TLDR; I'm in support of leaking ticks, if confusion exists, rename the method instead of overloading, could move the tick rate to implementation though.

This was merged into bleeding a long tim ago, and I believe is now in the 1.14.x branch. While my own thinking of this has changed over time, this is a stale discussion and the action proposed by this has already been done.

If it wants to be reversed, a new issue should be opened.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

LJNIC picture LJNIC  路  5Comments

Cybermaxke picture Cybermaxke  路  6Comments

BrainStone picture BrainStone  路  4Comments

ryantheleach picture ryantheleach  路  5Comments

mbax picture mbax  路  3Comments