The first thing you should do is run the code through a beautifier, because the indentations are very sus.
The link in that post leads to a page where you can paste a script, press a button and get the same script, but with correct indentation.
It might also be easier to right click the code in the script, pick Format and press Format Document to fix up the tabs. But the prettifier will also remove the trailing spaces at the ends of some lines.
Doing this will show that, for example, the for loops in Main starting at line 39 actually form a pyramid.
If there is nobody in PlayerRanks.Owner or PlayerRanks.admins2 etc., then there will be no check for whether a player is in PlayerRanks.banned, so that will not work at all. Furthermore, if there are two players with rank Owner, then any checks inside that pyramid are done twice. If there is one Owner, three admins2, 5 admins1 etc. and the player is banned, then
player:Kick("You Are Banned") will run 1 * 3 * 5 times.
There’s also a lot of code duplication everywhere.
One big example in Main is the Chatted connection.
You check for /e. If there is no /e, you do the commands. If there is /e, you remove the /e and do the commands.
You can cut off half the function by turning
if args ~= "/e" then
if args == "/e" then
This way, if you happen to change something in one half and forget to do it in the other, you won’t introduce any weird glitches when someone uses /e.
The code is really pyramidal in general.
You should turn these ifs that have nothing after them into sentinels.
if int.Value ~= math.huge then
if string.sub(args,1,1) == prefix then
if int.Value == math.huge then return end
if string.sub(args,1,1) ~= prefix then return end
This will remove a lot of indentations.
Another weird part is how
args.player is used.
You might need to turn
args.player into an array of matching players instead, and make all commands operate on pairs(array), and have a function that turns a string and the player who used the command, into a list of matching players.
If the function gets “me”, then it returns
If the function gets “all”, then it returns
If the function gets a name, then it returns all (or just one of) players whose name starts with it.
If the function gets “others”, then it returns
local players = `ServerPlayers:GetPlayers()`
table.remove(players, table.find(players, playerWhoUsedCommand))
The “all” mechanism is very prone to mistakes and crashing the game with infinite recursion.
The command list is also kind of ehh imo, because I know it’s possible to do better.
Each command could have some extra data associated with it that’s also returned with the command script.
The info could have a description of the command, for example.
Then the GUI could require the commands (or a separate module with just the info) and create a GUI with a list of commands based on that info.
This makes it so you don’t have to update both the GUI and the command list, reducing bugs and work needed and increasing consistency.