Pull remaining duration management out of EffectInstance to avoid surprising mutations for plugins

[dktapps]

Issue description

Plugin devs have been surprised by the fact that it's not possible to keep an EffectInstance around and reapply it to several mobs. This is a reasonable ask since it's possible to want to instantiate an effect with a predefined duration, strength etc. without having to make a new EffectInstance each time.

Of course, it is possible to simply clone the EffectInstance before using it, but this takes us right back to square one of the Effect type/instance merged model which is the whole reason why EffectInstance was introduced to begin with.

This issue proposes an ActiveEffectInstance type which will be used internally by EffectManager to track duration changes for each entity. It will contain the following methods moved from EffectInstance:

• getDuration()
• setDuration()
• decreaseDuration()
• hasExpired()

I'm not a big fan of the name, so I have also considered EffectDurationTracker, but I'm not very happy with that either.

Caveats

This adds yet another layer of abstraction to the Effect API, although one that won't affect developers very much. I've been avoiding this change for a long time for this exact reason.

It comes to my attention that Effect and EffectInstance might need renaming to make the intents of the different parts of this system more clear. Perhaps something like this:

• Effect -> EffectType
• EffectInstance -> EffectParameters (?)

I'm open to better naming suggestions.

[ghost]

no

[dktapps]

Probably better to just add a map<EffectInstance, duration> to the EffectManager, and then add a method like EffectManager::getRemainingDuration(Effect $effect) : int.

However, this would still be a behavioural BC break, because $effectManager->get($effect)->getDuration() would no longer reflect the remaining duration but instead the initial duration (and decreaseDuration() and setDuration() would have no effect).

While we're breaking BC, might as well make EffectInstance fully immutable too, to avoid all the kinds of headaches that come from doing this while forgetting to clone objects: https://github.com/pmmp/PocketMine-MP/blob/45917d495c14f3ce37bbf0848262ad8a470030f3/src/entity/projectile/SplashPotion.php#L117-L121