Use own ticker, prevents blocking bukkit threads

[Elecast2]

Use own ticker in its own Thread, prevents blocking Bukkit Threads

Now start() in SongPlayer adds it to a permanent Thread that loops every 10ms (I found this to be the higher delay without loosing song quality) then I removed the while with Thread.sleep that was in the old start() and changed it to an updateTick() function that checks if next song step should be played if enough time has passed.

This change allows many players to listen to different songs at the same time without blocking bukkit threads preventing other plugin's async functions to fail.

[sonarqubecloud]

Quality Gate failed

Failed conditions
1 Security Hotspot
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[koca2000]

Hi! Thank you for your PR!

BukkitScheduler uses an unlimited (Int.MAX_VALUE limited) cached thread pool so the fact that there is one thread for each SongPlayer doesn't block any Bukkit threads. The solution with a single thread makes every new SongPlayer dependent on the execution of all the previous ones which may hurt timings. Also this way the tickrate is hard limited to 100 TPS and in practice it would of course be less.

other plugin's async functions to fail.

Would you elaborate? What issues have you observed? The problem may be in fact with synchronization to the server's main thread. This synchronization is going to be removed in NoteBlockAPI 2.0.

[Elecast2]

Hello, thanks for answering. Our custom spigot uses limited Craft Scheduler Threads, we didn't know this plugin relies on unlimited bukkit threads, in our case since the Threads are limited, it only takes just a few players listening to different songs to freeze the threads preventing other stuff from working properly. I feel like using custom threads with ticker is still better providing more control, I only added one but it can be easily updated to use a Thread Pool with fixed amount.

About the 10 ms delay, it can be also changed, but I wasn't able to notice any song timing quality loss with that delay.

[koca2000]

I was thinking about it and I don't want to do a change of this critical part of the API in v1.x because it already has a its timing issues and I don't want to add more. Also I want to focus on the rewrite. I created an issue #120 to track it and I'll modify the v2.0 rewrite version to use some SongTicker.