SkinApplier now only applies a skin if a player doesn't already have one

[Teamplayer]

When applying skins to Floodgate players there is a chance of overwriting a skin that another plugin intentionally applied. With this change, all SkinAppliers now check to make sure a player has an empty or null skin before applying a skin to them. This prevents Floodgate from applying a Bedrock skin over a skin that was already set by another plugin.

[Teamplayer]

While in practice the skin applier only has to apply skins when the Bedrock player doesn't already have a skin, I still think that the check code shouldn't be in the applySkin method.

I suggest making a separate method in the SkinApplier classes called something like hasSkin(FloodgatePlayer) which checks if the player has textures, and that method has to be called when Floodgate wants to apply a skin.

I agree. This was actually my original implementation idea, but I was scared to modify SkinApplier for some reason.

The only issue with this implementation is that in SpigotSkinApplier it's implied that sometimes applySkin is called before the Player object is created. The solution to this is to wait 8 minutes (I'm assuming it's supposed to be 10 seconds), and then try again. If I were to make changes to always check hasSkin before applySkin there would be a condition in Spigot where I could not definitely say if the player has a skin or not as the Player object wouldn't exist. Also, when applySkin retries after a delay, it would have to check hasSkin again.

I'll work on a solution, but any advice would be appreciated on how to approach solving that one issue.

[Teamplayer]

The only easy way around the issue was to use the hasSkin method inside of the applySkin method when it was retrying in SpigotSkinApplier. I don't think it's preferable, but I think it may be better than the larger refactoring needed.

[Tim203]

I forgot about the Spigot workaround. Will have to take a closer look at why we are doing that null check again.

Can you btw remove the optional and replace the streams with for loops? We use null checks everywhere in Floodgate and there is only one stream.

The solution to this is to wait 8 minutes (I'm assuming it's supposed to be 10 seconds)

I probably forgot that Bukkit uses ticks everywhere 🙃 Feel free to fix that in this PR ^^

[Tim203]

Honestly, the stream stuff is not a big deal. It's fine for now (would be nice to have the delay fixed though). I'll give an update when I have an update on the Spigot workaround.