File NPE Fix

[TrollsterCooleg]

This pull request is a fix for https://github.com/dsdd/EnchantmentsPlusMinus/issues/51

First of all, changed the getUuid to make more sense and not be dependent on some sort of wierd uuid store, and sticks to the Bukkit API. Also restructured the getUserDataFile function to use brackets, as that makes the code make a lot more sense and just generally follow standards and not feel like it will always return null. All functionality of whatever was there before was restored, including returning null if the player hasnt logged on before.

[Geolykt]

I myself believe that the exception can only occur when the player changes it's name while it is logged-in. That would probably also break other system in the meanwhile. If this does however actually fix the issue, it is imperative to test whether one could feasibly (D-)DOS the server by running the command a lot of times. Chat is async, but I don't think commands are.

[rackgaming]

I can confirm that I've always had the same name and have never ever changed my name and my UUID has always been the same UUID.

[Geolykt]

What I'm saying is that if the plugin breaks, so should bukkit. After further looking at the code I believe that the issue is caused by invoking /tokens userthatdoesntexistwhichcausesthenpe, this PR does not mitigate that.

[rackgaming]

the words are confusing here so I'll stay out of it but my UUID and username does exist in the usernamestore.yml as well as the UUID.yml files correlating that I have tokens as well in those .yml files.

[TrollsterCooleg]

It seems like it could fix it not functioning on valid users, it seems like just a wierd system being used right now and ill be completely honest I don't understand why it wasnt done like this from the start. Even if it doesnt fix the exception, the code makes like 50x more sense, and is far more readable (at least in my opinion)

[Geolykt]

If the player does not exist, it breaks. In #51 the reporter says that he ran /tokens bank. Which means the UUID of the player bank is fetched. That player does not exist, which means that the UUID under the old system is null. After even more thought (haven't been doing bukkit for a few months now) I believe that your proposed changes do indeed solve the issues on that front as the returned UUID would be something along the lines of UUID.fromBytes(("OfflinePlayer: " + playerName).getBytes(StandardCharsets.UTF_8)). My performance worries remain as https://hub.spigotmc.org/javadocs/spigot/org/bukkit/Bukkit.html#getOfflinePlayer(java.lang.String) defines the method to possibly "involve a blocking web request".

[rackgaming]

MineHut uses offline-mode which explains that it needs to get an offline player / cracked player. which I get ppl don't support cracked or cheaters but that's where we use anti-cheat plugins.

[Geolykt]

Nah, that is not the issue.

[rackgaming]

however I did not know that "tokens bank" itself was not a part of your plugin so I was not trying to get the username "bank" to give me information. I clearly got confused with another plugin but as far as the command goes, it does work fine. sorry for the confusion

[Geolykt]

It seems like it could fix it not functioning on valid users, it seems like just a wierd system being used right now and ill be completely honest I don't understand why it wasnt done like this from the start. Even if it doesnt fix the exception, the code makes like 50x more sense, and is far more readable (at least in my opinion)

Such a system does indeed make sense, but generally only in the reverse. Quite a few plugins have UUID -> name caches to avoid situations where the player cache expires and where the name of the player cannot be resolved. However in this specific case of fetching the UUID based on names, it can be more sensible to use Bukkit#getOfflinePlayer to obtain the UUID - for as long as the request is done asynchronously. I believe that it is actually possible to do just that, although the token economy would need to be rewritten in order to allow async requests (why the fucc is TokenEconomy#getBalance not cached?).

[dsdd]

i have attempted addressing this issue in 1.9r-14, though i do not know if this will work or not since well i cant really test it and the Also restructured the getUserDataFile function to use brackets, as that makes the code make a lot more sense and just generally follow standards and not feel like it will always return null i dont know what you mean but well the code is gone now 👍

[rackgaming]

I will test it later when I get back home from pneumia and flu shot

[dsdd]

dont see any need for this pull