Refactor command requirement checks (#538)

AND-ing requirements instead of OR-ing them is more intuitive and less prone to misunderstanding
abalabahaha · Oct 13, 2019 · 07d24f5 · 07d24f5
1 parent 6702773
commit 07d24f5
Showing 1 changed file with 19 additions and 40 deletions.
diff --git a/lib/command/Command.js b/lib/command/Command.js
@@ -164,73 +164,52 @@ class Command {
     }
 
     permissionCheck(msg) {
-        let req = false;
-        if(this.requirements.custom && typeof this.requirements.custom === "function") {
-            req = true;
-            if(this.requirements.custom(msg)) {
-                return true;
-            }
+        if(this.requirements.custom && typeof this.requirements.custom === "function" && !this.requirements.custom(msg)) {
+            return false;
         }
-        if(typeof this.requirements.userIDs === "function") {
-            req = true;
-            if(this.requirements.userIDs(msg).includes(msg.author.id)) {
-                return true;
-            }
-        } else if(this.requirements.userIDs.length > 0) {
-            req = true;
-            if(this.requirements.userIDs.includes(msg.author.id)) {
-                return true;
+        if (this.requirements.userIDs) {
+            const userIds = typeof this.requirements.userIDs === "function" ? this.requirements.userIDs(msg) : this.requirements.userIDs;
+            if(userIds.length > 0 && !this.requirements.userIDs.includes(msg.author.id)) {
+                return false;
             }
         }
-        if(!msg.channel.guild) {
-            return !this.guildOnly && !req;
-        } else if(this.dmOnly) {
+        if(msg.channel.guild) {
+            if(this.dmOnly) {
+                return false;
+            }
+        } else if(this.guildOnly) {
             return false;
         }
         const keys = typeof this.requirements.permissions === "function" ? Object.keys(this.requirements.permissions(msg)) : Object.keys(this.requirements.permissions);
         if(keys.length > 0) {
-            req = true;
             const permissions = msg.channel.permissionsOf(msg.author.id).json;
             for(const key of keys) {
                 if(this.requirements.permissions[key] !== permissions[key]) {
-                    req = false;
-                    break;
+                    return false;
                 }
             }
-            if(req) {
-                return true;
-            }
-            req = true;
         }
         if(msg.member) {
             let roles = msg.member.roles || [];
             if(this.requirements.roleIDs) {
-                req = true;
-                let requiredRoleIDs = this.requirements.roleIDs;
-                if(typeof requiredRoleIDs === "function") {
-                    requiredRoleIDs = this.requirements.roleIDs(msg);
-                }
+                const requiredRoleIDs = typeof requiredRoleIDs === "function" ? this.requirements.roleIDs(msg) : this.requirements.roleIDs;
                 for(const roleID of requiredRoleIDs) {
-                    if(roles.includes(roleID)) {
-                        return true;
+                    if(!roles.includes(roleID)) {
+                        return false;
                     }
                 }
             }
             if(this.requirements.roleNames) {
-                req = true;
                 roles = roles.map((roleID) => msg.channel.guild.roles.get(roleID).name);
-                let requiredRoleNames = this.requirements.roleNames;
-                if(typeof requiredRoleNames === "function") {
-                    requiredRoleNames = this.requirements.roleNames(msg);
-                }
+                const requiredRoleNames = requiredRoleNames === "function" ? this.requirements.roleNames(msg) : this.requirements.roleNames;
                 for(const roleName of requiredRoleNames) {
-                    if(roles.includes(roleName)) {
-                        return true;
+                    if(!roles.includes(roleName)) {
+                        return false;
                     }
                 }
             }
         }
-        return !req;
+        return true;
     }
 
     cooldownExclusionCheck(msg) {