Closed forkiesassds closed 3 years ago
I don't really understand the purpose of this, but if I'm understanding correctly, then it allows multiple people to have their permissions changed at the same time? You should update the help of the commands to reflect this expansion of syntax. But it seems unnecessary to me
EDIT: I only just noticed this is addressing an existing issue, so I guess that answers my question of what the purpose is
Unfortunately several issues here
/perbuild -
throws an IndexedOutOfRangeException now, instead of messaging the player You must provide a player name to blacklist.
if (grp == null) hasErrors = true; errorCount++;
is really equivalent to if (grp == null) { hasErrors = true; } errorCount++;
not if (grp == null) { hasErrors = true; errorCount++; }
, so errorCount will usually be incorrectaccess.SetMin/Max
still gets called with group
being null
, therefore causing a NullReferenceExceptionOne or more errors occurred while executing this command.
message is necessary, since the command already messages you about each invalid rank/player nameaccess.Blacklist(p, data.Rank, lvl, access.Whitelisted[0]);
logic is problematic, because if you can't blacklist a whitelisted player (e.g. due to them being a higher rank), the code will just loop trying to remove that player (and failing each time), instead of moving onto the subsequent playerNot an issue with this pull request specifically, but I'm a little bit reluctant towards providing a way for someone to easily clear an
entire whitelist with just -*
I apologise if this comes off as overly critical
Might need some improvements.
Closes #368.