Xitee1 / PowerBoard

Minecraft plugin for Scoreboard, Tablist, Prefixes, Suffixes, Chat | Animated
GNU General Public License v3.0
11 stars 4 forks source link

Tablist placeholder refreshing #15

Open Xitee1 opened 2 years ago

Xitee1 commented 2 years ago

The tablist will always update with the speed of the lowest speed number of any line. If for example a placeholder in a tablist line has a speed of 100, it will update every 5 seconds. But if there is another line with a placeholder that has a speed of 5 which updates 4 times in a second, the first placeholder will also update 4 times a second instead all 5 seconds.

NicoNekoDev commented 2 years ago

https://github.com/Xitee1/PowerBoard/blob/2da5e02139d46f6481fcddcf7b5cbd76ce00ec5c/src/de/xite/scoreboard/modules/tablist/TablistManager.java#L108 The speed can be -1 and Math.min would return -1 which goes into the scheduler, and according to spigot documentation/source code, a scheduler with interval -1 would run once which is why the bug happening, and the refreshing doesn't work. One simple way to fix it: if (speed >= 0 && speed < 9999) { interval = Math.min(interval, speed); }

Xitee1 commented 2 years ago

Thanks @NicoNekoDev This will not exactly fix the problem which is described in this issue (You probably didn't understand it correctly) but it will actually fix another problem that I didn't even think about.

NicoNekoDev commented 2 years ago

Okay, I was wrong. The actual fix it would be to set the interval to 1 if is lower than 1. Yeah, it fixes the refreshes not working on tablist.

Xitee1 commented 2 years ago

The actual fix it would be to set the interval to 1 if is lower than 1

No, the code that you've poseted is correct, because -1 is meant to disabling refreshing for that line

tablist.yml description:

If you have static scores (no animations or updates needed): Set the 'speed' value to '-1' or '9999'. Then the scheduler won't start to save performance.

NicoNekoDev commented 2 years ago

Yes, my code is correct, but after a bit of review, I came to conclusion that doing if (speed < 1) interval = 1 because: 1) it doesn't go higher than the set interval (20 * 60) 2) it only requires to not go bellow 1, not below or equal to 0

Xitee1 commented 2 years ago

That's correct, it doesn't need to check if it is lower than 9999. But if the speed is below 0, it should be just ignored because everything below 0 is a static line which does not need to be updated and therefore the tablist would update each tick unnecessarily if the interval is set to 1.

Xitee1 commented 2 years ago

However, I have changed the code and just made the startXXAnimation functions a boolean which returns if the scheduler has been started or not (It does not start, if the speed is below 0 or higher than 9999). If it has started it executes Math.min, otherwise it does not. https://github.com/Xitee1/PowerBoard/commit/051bb836e60e647b9db9e20ef57df6c6c375a621

Xitee1 commented 1 year ago

Sadly there seems to be no easy fix for this. One way to reduce the unnecessary calls would be to split header and footer updates. A better solution might be to cache the lines with the text already replaced with placeholders so it only needs to update/replace the placeholders that are actually needed.