search

jcdesimp / Landlord

Bukkit land claiming plugin.
MIT License
11 stars 21 forks source link

add checks for option particleEffects before trying to resolve the Effect enum; should solve #48 #32

Closed colbin8r closed 8 years ago

colbin8r commented 9 years ago

See http://dev.bukkit.org/bukkit-plugins/landlord/tickets/48-internal-error-when-using-any-landlord-command-no-world/ for more details.

jcdesimp commented 9 years ago

@colbin8r Been a while since I looked at that code, however I feel like its a workaround for the true problem. On this line the config is already being checked. https://github.com/jcdesimp/Landlord/blob/develop/src/com/jcdesimp/landlord/persistantData/OwnedLand.java#L411

The real issue to investigate is why the enum is not resolving. It's been a while since I touched these areas of the code but if I remember correctly the enum is spigot exclusive, doesn't exist in vanilla bukkit. This user appears to be running craftbukkit. I did officially say somewhere that Landlord now works only on Spigot, although to be quite honest, this particle stuff is literally the only reason. If I weren't so busy at the moment I would like to fix this so that it works on bukkit again because I'm sure its a nuisance for some users. This would technically fix the issue but only kind of. The plugin will still break for users who are using bukkit and attempt to have particle effects enabled.

One possible solution is to abstract away the particle effect and having separate implementations for bukkit and spigot, perhaps not actually implementing the particles for bukkit initially and instead not supporting the feature when running on bukkit and letting users know the particular feature works on spigot only.

Battelman2 commented 8 years ago

This will be reevaluated for Landlord 1.4. I am archiving this pull request, and we can open a new one for configuration changes such as yours.