Adds Spark Support!

[EquipableMC]

Description

This PR simply adds features from Spark into Skript!

Target Minecraft Versions: 1.19+
Requirements: Spark plugin
Related Issues: none

[Moderocky]

I'm sure you have enough comments to work on already so I won't add anything to that.
I think this is a nice idea, but I am a little concerned by the 'weirdness' of Spark's actual statistics collection & implementation. It feels like the stats that can be provided are pretty limited and I'm not sure how much a user will be able to do with this information (aside from printing out exactly what they're given).
Do you think this might be better for an addon?

[EquipableMC]

I'm sure you have enough comments to work on already so I won't add anything to that.

I think this is a nice idea, but I am a little concerned by the 'weirdness' of Spark's actual statistics collection & implementation. It feels like the stats that can be provided are pretty limited and I'm not sure how much a user will be able to do with this information (aside from printing out exactly what they're given).

Do you think this might be better for an addon?

I don't think this should be done as an addon since spark is built into most server jars (other than spigot)

[Sparky200]

A couple strange things from design standpoint and other small things.

[Sparky200]

Would it be better for this condition to be performed by MsptStatisticType? i.e. have a method in there that takes in the info and the value.

Here, that would simply be:

Arrays.stream(infos).map(info -> type.getProperty(info, value)).toArray(Double[]::new);

The enum, getProperty(...):

if (function != null) return function.apply(info)
return percentileFunction.apply(info)

Just an ownership concern (the enum should generally own and control how its own fields are used).
Also, while the enum is closed, it would still benefit from having null safety around percentileFunction and function. Especially in this scenario, it took me a while to realize there's a contractual reason it won't run into a null check.

[Sparky200]

Maybe it's just me but it's a little scary when a toString could throw an exception

[EquipableMC]

Well what should I put in the default then?

[Sparky200]

This may be something to quickly ask the team whether they prefer keeping exceptions out of "special" methods like toString(). My reasoning is that it helps immensely with debugging if I can always view a quick representation of an object in a debugger.

[Sparky200]

I think this class would benefit from having a better lifecycle, mainly to orchestrate its state a bit better.
What I recommend is adding and refactoring the following:

• rename to something else. This is hardly a utility class, it's a stateful singleton.
• Add an initialize() method that consumes the desired Statistic classes (process cpu usage, system cpu usage, mspt, tps)
• Add a dispose() method that de-initializes the state and prepares this singleton to be re-initialized at another time
• Manage this lifecycle from somewhere, probably SparkModule. Ultimately, onEnable should call initialize() and onDisable should call dispose()

This will clean up pretty much all of your synchronized blocks, which barely needed to be there in the first place.

[EquipableMC]

I don't even know how to do all of that, but it works fine as it is, so I will leave it.

[Absolutionism]

I don't think you should be resolving conflicts after you reply when it seems a discussion should be made. Rather than "I can't/won't do this" then resolve it.

[EquipableMC]

I don't think you should be resolving conflicts after you reply when it seems a discussion should be made. Rather than "I can't/won't do this" then resolve it.

I only marked them as resolved as it works fine on it's own and with my almost a year of java experience, I only know so much and figured it was fine the way it was. I left some unresolved as it requires Sovde's comment as this PR was entirely recoded by Sovde and I don't really understand his thinking/thought process in some of this even when looking at the code over and over again.
Edit: I have unmarked some that I found where a discussion where be made. I forgot to mention that, oops.

[EquipableMC]

I am marking this PR as draft incase someone else wants to take this over and better improve the PR. Once that happens, I will close this PR. You can view the entire conversation here.

[EquipableMC]

Update:
I am marking this PR as closed. Someone can take it over or it can be done in a seperate addon. You can view the entire conversation here.