Fix privileges commands (cleanup)

src/main/kotlin/io/dico/parcels2/Privilege.kt

@@ -74,6 +74,12 @@ interface PrivilegesMinimal {
 interface Privileges : PrivilegesMinimal {
     val keyOfOwner: PlayerProfile.Real?
 
+    fun privilege(player: OfflinePlayer): Privilege {
+        val key = player.privilegeKey
+        return if (key == keyOfOwner) OWNER
+        else getStoredPrivilege(key)
+    }
+
     fun privilege(player: OfflinePlayer, adminPerm: String): Privilege =
         if (player is Player && player.hasPermission(adminPerm)) ADMIN
         else {

src/main/kotlin/io/dico/parcels2/command/AbstractParcelCommands.kt

@@ -4,6 +4,7 @@ import io.dico.dicore.command.*
 import io.dico.parcels2.ParcelWorld
 import io.dico.parcels2.ParcelsPlugin
 import io.dico.parcels2.PlayerProfile
+import io.dico.parcels2.PrivilegeChangeResult
 import io.dico.parcels2.util.ext.hasPermAdminManage
 import io.dico.parcels2.util.ext.parcelLimit
 import org.bukkit.entity.Player
@@ -52,6 +53,10 @@ abstract class AbstractParcelCommands(val plugin: ParcelsPlugin) : ICommandRecei
             }
     }
 
+    protected fun err(message: String): Nothing = throw CommandException(message)
+
     override fun getCoroutineContext() = plugin.coroutineContext
+
+
 }
 

src/main/kotlin/io/dico/parcels2/command/CommandsPrivilegesGlobal.kt

@@ -1,12 +1,11 @@
+@file:Suppress("NON_EXHAUSTIVE_WHEN")
+
 package io.dico.parcels2.command
 
-import io.dico.dicore.command.Validate
 import io.dico.dicore.command.annotation.Cmd
 import io.dico.dicore.command.annotation.Desc
-import io.dico.parcels2.GlobalPrivileges
-import io.dico.parcels2.GlobalPrivilegesManager
 import io.dico.parcels2.ParcelsPlugin
-import io.dico.parcels2.PlayerProfile
+import io.dico.parcels2.PrivilegeChangeResult.*
 import org.bukkit.OfflinePlayer
 import org.bukkit.entity.Player
 
@@ -19,11 +18,12 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "Allows a player to manage this parcel",
         shortVersion = "allows a player to manage this parcel"
     )
-    fun cmdEntrust(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdEntrust(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(player != sender, "The target cannot be yourself")
+        when (data[sender].allowManage(player)) {
-        Validate.isTrue(data[sender].allowManage(player), "${player.name} is already allowed to manage globally")
+            FAIL_OWNER -> err("The target cannot be yourself")
-        return "${player.name} is now allowed to manage globally"
+            FAIL -> err("${player.name} is already allowed to manage globally")
-    }
+            SUCCESS -> "${player.name} is now allowed to manage globally"
+        }
 
     @Cmd("distrust")
     @Desc(
@@ -31,10 +31,12 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "they will still be able to build",
         shortVersion = "disallows a player to manage globally"
     )
-    fun cmdDistrust(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdDistrust(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(data[sender].disallowManage(player), "${player.name} is not currently allowed to manage globally")
+        when (data[sender].disallowManage(player)) {
-        return "${player.name} is not allowed to manage globally anymore"
+            FAIL_OWNER -> err("The target cannot be yourself")
-    }
+            FAIL -> err("${player.name} is not currently allowed to manage globally")
+            SUCCESS -> "${player.name} is not allowed to manage globally anymore"
+        }
 
     @Cmd("allow", aliases = ["add", "permit"])
     @Desc(
@@ -42,11 +44,12 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "the parcels that you own.",
         shortVersion = "globally allows a player to build on your parcels"
     )
-    fun cmdAllow(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdAllow(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(player != sender, "The target cannot be yourself")
+        when (data[sender].allowBuild(player)) {
-        Validate.isTrue(data[sender].allowBuild(player), "${player.name} is already allowed globally")
+            FAIL_OWNER -> err("The target cannot be yourself")
-        return "${player.name} is now allowed to build on all your parcels"
+            FAIL -> err("${player.name} is already allowed globally")
-    }
+            SUCCESS -> "${player.name} is now allowed to build on all your parcels"
+        }
 
     @Cmd("disallow", aliases = ["remove", "forbid"])
     @Desc(
@@ -56,11 +59,12 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "parcels, they can still build there.",
         shortVersion = "globally disallows a player to build on your parcels"
     )
-    fun cmdDisallow(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdDisallow(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(player != sender, "The target cannot be yourself")
+        when (data[sender].disallowBuild(player)) {
-        Validate.isTrue(data[sender].disallowBuild(player), "${player.name} is not currently allowed globally")
+            FAIL_OWNER -> err("The target cannot be yourself")
-        return "${player.name} is not allowed to build on all your parcels anymore"
+            FAIL -> err("${player.name} is not currently allowed globally")
-    }
+            SUCCESS -> "${player.name} is not allowed to build on all your parcels anymore"
+        }
 
     @Cmd("ban", aliases = ["deny"])
     @Desc(
@@ -68,11 +72,12 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "that you own, making them unable to enter.",
         shortVersion = "globally bans a player from your parcels"
     )
-    fun cmdBan(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdBan(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(player != sender, "The target cannot be yourself")
+        when (data[sender].ban(player)) {
-        Validate.isTrue(data[sender].ban(player), "${player.name} is already banned from all your parcels")
+            FAIL_OWNER -> err("The target cannot be yourself")
-        return "${player.name} is now banned from all your parcels"
+            FAIL -> err("${player.name} is already banned from all your parcels")
-    }
+            SUCCESS -> "${player.name} is now banned from all your parcels"
+        }
 
     @Cmd("unban", aliases = ["undeny"])
     @Desc(
@@ -82,9 +87,11 @@ class CommandsPrivilegesGlobal(plugin: ParcelsPlugin) : AbstractParcelCommands(p
         "they will still be banned there.",
         shortVersion = "globally unbans a player from your parcels"
     )
-    fun cmdUnban(sender: Player, player: OfflinePlayer): Any? {
+    fun cmdUnban(sender: Player, player: OfflinePlayer): Any? =
-        Validate.isTrue(data[sender].unban(player), "${player.name} is not currently banned from all your parcels")
+        when (data[sender].unban(player)) {
-        return "${player.name} is not banned from all your parcels anymore"
+            FAIL_OWNER -> err("The target cannot be yourself")
-    }
+            FAIL -> err("${player.name} is not currently banned from all your parcels")
+            SUCCESS -> "${player.name} is not banned from all your parcels anymore"
+        }
 
 }

src/main/kotlin/io/dico/parcels2/command/CommandsPrivilegesLocal.kt

@@ -5,6 +5,7 @@ import io.dico.dicore.command.annotation.Cmd
 import io.dico.dicore.command.annotation.Desc
 import io.dico.parcels2.ParcelsPlugin
 import io.dico.parcels2.Privilege
+import io.dico.parcels2.PrivilegeChangeResult.*
 import io.dico.parcels2.util.ext.hasPermAdminManage
 import org.bukkit.OfflinePlayer
 import org.bukkit.entity.Player
@@ -19,9 +20,12 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     @RequireParcelPrivilege(Privilege.OWNER)
     fun ParcelScope.cmdEntrust(sender: Player, player: OfflinePlayer): Any? {
         Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
-        Validate.isTrue(!parcel.owner!!.matches(player), "The target already owns the parcel")
+
-        Validate.isTrue(parcel.allowManage(player), "${player.name} is already allowed to manage this parcel")
+        return when (parcel.allowManage(player)) {
-        return "${player.name} is now allowed to manage this parcel"
+            FAIL_OWNER -> err("The target already owns the parcel")
+            FAIL -> err("${player.name} is already allowed to manage this parcel")
+            SUCCESS -> "${player.name} is now allowed to manage this parcel"
+        }
     }
 
     @Cmd("distrust")
@@ -32,8 +36,13 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     )
     @RequireParcelPrivilege(Privilege.OWNER)
     fun ParcelScope.cmdDistrust(sender: Player, player: OfflinePlayer): Any? {
-        Validate.isTrue(parcel.disallowManage(player), "${player.name} is not currently allowed to manage this parcel")
+        Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
-        return "${player.name} is not allowed to manage this parcel anymore"
+
+        return when (parcel.disallowManage(player)) {
+            FAIL_OWNER -> err("The target owns the parcel and can't be distrusted")
+            FAIL -> err("${player.name} is not currently allowed to manage this parcel")
+            SUCCESS -> "${player.name} is not allowed to manage this parcel anymore"
+        }
     }
 
     @Cmd("allow", aliases = ["add", "permit"])
@@ -44,10 +53,13 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     @RequireParcelPrivilege(Privilege.CAN_MANAGE)
     fun ParcelScope.cmdAllow(sender: Player, player: OfflinePlayer): Any? {
         Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
-        Validate.isTrue(!parcel.owner!!.matches(player), "The target already owns the parcel")
         Validate.isTrue(parcel.privilege(sender) > parcel.privilege(player), "You may not change the privilege of ${player.name}")
-        Validate.isTrue(parcel.allowBuild(player), "${player.name} is already allowed to build on this parcel")
+
-        return "${player.name} is now allowed to build on this parcel"
+        return when (parcel.allowBuild(player)) {
+            FAIL_OWNER -> err("The target already owns the parcel")
+            FAIL -> err("${player.name} is already allowed to build on this parcel")
+            SUCCESS -> "${player.name} is now allowed to build on this parcel"
+        }
     }
 
     @Cmd("disallow", aliases = ["remove", "forbid"])
@@ -58,9 +70,14 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     )
     @RequireParcelPrivilege(Privilege.CAN_MANAGE)
     fun ParcelScope.cmdDisallow(sender: Player, player: OfflinePlayer): Any? {
+        Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
         Validate.isTrue(parcel.privilege(sender) > parcel.privilege(player), "You may not change the privilege of ${player.name}")
-        Validate.isTrue(parcel.disallowBuild(player), "${player.name} is not currently allowed to build on this parcel")
+
-        return "${player.name} is not allowed to build on this parcel anymore"
+        return when (parcel.disallowBuild(player)) {
+            FAIL_OWNER -> err("The target owns the parcel")
+            FAIL -> err("${player.name} is not currently allowed to build on this parcel")
+            SUCCESS -> "${player.name} is not allowed to build on this parcel anymore"
+        }
     }
 
     @Cmd("ban", aliases = ["deny"])
@@ -72,10 +89,13 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     @RequireParcelPrivilege(Privilege.CAN_MANAGE)
     fun ParcelScope.cmdBan(sender: Player, player: OfflinePlayer): Any? {
         Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
-        Validate.isTrue(!parcel.owner!!.matches(player), "The owner cannot be banned from the parcel")
         Validate.isTrue(parcel.privilege(sender) > parcel.privilege(player), "You may not change the privilege of ${player.name}")
-        Validate.isTrue(parcel.ban(player), "${player.name} is already banned from this parcel")
+
-        return "${player.name} is now banned from this parcel"
+        return when (parcel.disallowBuild(player)) {
+            FAIL_OWNER -> err("The target owns the parcel")
+            FAIL -> err("${player.name} is already banned from this parcel")
+            SUCCESS -> "${player.name} is now banned from this parcel"
+        }
     }
 
     @Cmd("unban", aliases = ["undeny"])
@@ -86,9 +106,14 @@ class CommandsPrivilegesLocal(plugin: ParcelsPlugin) : AbstractParcelCommands(pl
     )
     @RequireParcelPrivilege(Privilege.CAN_MANAGE)
     fun ParcelScope.cmdUnban(sender: Player, player: OfflinePlayer): Any? {
+        Validate.isTrue(parcel.owner != null || sender.hasPermAdminManage, "This parcel is unowned")
         Validate.isTrue(parcel.privilege(sender) > parcel.privilege(player), "You may not change the privilege of ${player.name}")
-        Validate.isTrue(parcel.unban(player), "${player.name} is not currently banned from this parcel")
+
-        return "${player.name} is not banned from this parcel anymore"
+        return when (parcel.disallowBuild(player)) {
+            FAIL_OWNER -> err("The target owns the parcel")
+            FAIL -> err("${player.name} is not currently banned from this parcel")
+            SUCCESS -> "${player.name} is not banned from this parcel anymore"
+        }
     }
 
 }