Add boss bar animation when it appears

[IamMusavaRibica]

Here is a brief summary of the changes:

• Removed code duplication: calculating progress as double and casting it to float in bossBar#progress(float) method.

double progress = currentXp / levelXp;
if (progress > 1 || progress < 0) {
    progress = 1.0;
}

• Changed signature of methods handleNewBossBar andhandleExistingBossBar to take progress instead of currentXp and levelXp, and progress as float instead of double
• Added a comment that explains it

note that the first commit bf80e34 should have been amended but git complained with merge conflicts, so it looks like it got duplicated at 10c922e

[Archy-X]

Is the animation really needed in the case where the player already has the boss bar visible? Because it already "animates" between progress when the previous XP amount is already showing. Maybe add some way to detect if there's one actively showing for that skill and set the progress immediately?

[IamMusavaRibica]

Is the animation really needed in the case where the player already has the boss bar visible?

Maybe. It would require an if in the handleExistingBossBar method. BossBarManager keeps a reference to distinct boss bars for every player and skill, and when the player doesn't actively see it, it's not deleted and recreated later, it is created only once for every player and skill. That's why it can also "appear" even in handleExistingBossBar.

Maybe add some way to detect if there's one actively showing for that skill and set the progress immediately?

The only way to check is using the data in currentActions and checkCurrentActions

[Archy-X]

I just added an option to toggle it off since detecting if its showing wasn't as straightforward as I thought. Can you verify that it is still working as intended?

[IamMusavaRibica]

I just added an option to toggle it off since detecting if its showing wasn't as straightforward as I thought. Can you verify that it is still working as intended?

Unfortunately it does not, you didn't account for the fact that BossBarManager is reused after /skill reload, not recreated, so if you change the boss_bar.animate_progress boolean in the config, it will not update. I decided not to touch ReloadExecutor for this, and instead modified BossBarManager's methods for handling boss bars - so that the option is looked up every time the method is called. I'm sure it doesn't have large performance deficiency.

[IamMusavaRibica]

In 584eecd I just changed code flow and comments to a way I feel would be easier to comprehend

[Archy-X]

I actually made ANIMATE_PROGRESS a class field that didn't update on reload on purpose so it would be slightly more performant, especially since the boss bar is known to be a source of lag on larger servers. I would prefer if it was set back to a class field and updated through the existing BossBarManager#loadOptions method which is already called by ReloadExecutor.

[IamMusavaRibica]

What's the reasoning behind making it final then? I don't know why one would have to restart the entire server instead of just the plugin in order to update boss_bar.animate_progress option, but I have also never came across a server where boss bar causes performance issues.

[Archy-X]

You can make it non-final and update it in loadOptions.