Simple Admin Script

Help and Feedback Code Review
#1

I’m looking for feedback on my code.

ServerScript

local server = require(script.MainModule)
game.Players.PlayerAdded:Connect(function(player)
	local settings = {
		["General"] = {
			Prefix = ":"
		},
		["Ranks"] = {
			Moderators = {},
			Administrators = {},
			Owners = {},
			Creators = {"blvckjakey"},
		}
		
	}
	spawn(function()
		server.run(player, settings)
	end)
end)

Module

local admin = {}

admin.run = function(...)
	local arguments = {...};
	local player = arguments[1]
	local settings = arguments[2]
	local prefix = settings.General.Prefix
	local function isAdmin(target, level)
		local rank = string.lower(level)
		if rank == "moderator" then
			for i,v in pairs(settings.Ranks.Moderators, settings.Ranks.Administrators, settings.Ranks.Owners, settings.Ranks.Creators) do
				if target == v then
					return true
				else
					return false
				end
			end
		end
		if rank == "admin" then
			for i,v in pairs(settings.Ranks.Administrators, settings.Ranks.Owners, settings.Ranks.Creators) do
				if target == v then
					return true
				else
					return false
				end
			end
		end
		if rank == "owner" then
			for i,v in pairs(settings.Ranks.Owners, settings.Ranks.Creators) do
				if target == v then
					return true
				else
					return false
				end
			end
		end
		if rank == "creator" then
			for i,v in pairs(settings.Ranks.Creators) do
				if target == v then
					return true
				else
					return false
				end
			end
		end
	end
	local function findTarget(input)
		input = tostring(input):lower()
		for _, player in ipairs(game:GetService("Players")) do
			if (player.Name:lower():sub(1, #input) == input) then
				return player
			else
				return "Player couldn't be found"
			end
		end
	end
	local function adminLog(command, admin, target)
		warn(command.." ran the command ".. command .." on ".. target)
	end
	player.Chatted:Connect(function(message)
		local msg = string.lower(message)
		if msg:sub(1, 8) == prefix.."respawn" then
			if isAdmin(player.Name, "Moderator") then
			local subLower = string.lower(msg:sub(9))
			if subLower == "me" then
				player:LoadCharacter()
			else
			local target = game.Players:FindFirstChild(msg:sub(9)) 
			if target == nil then
					if findTarget(msg:sub(9)) == "Player couldn't be found" then
							warn("Player could'nt be found")
						else
					local tempTarget = findTarget(msg:sub(9))
					tempTarget:LoadCharacter()
				end
			else
				target:LoadCharacter()
				end
				end
			else
				warn("Player isnt a admin, can't run this command")
			end
		end
	end)
end

return admin
3 Likes
#2

It looks good! If all you’re gonna use the General table for is the prefix, you can actually remove that and just make prefix a variable.

1 Like
#3

(accidentally submitted, will continue to edit)

  1. The indentation is really bad in the second script, and it makes it hard to read. Run your script through a beautifier if you don’t want to fix it yourself.
  2. You’re creating a new settings table for each player that joins. Move that to the top of the script, or into its own Settings ModuleScript.
  3. admin.run should take explicit player and settings arguments, not ....
  4. admin.run does a lot of unnecessary work for each player. All of those helper methods, isAdmin, findTarget, adminLog, etc. should be top-level functions. There’s no reason to duplicate them for each new call to admin.run.
  5. Similarly to the last point, it would be more readable if you had a separate OnChatted(player, message) function that you called like: player.Chatted:Connect(function(message) OnChatted(player, message) end)
  6. isAdmin is wrong, since it will return true or false after the first person in a list. The loops should look like this:
    for i, v in pairs(--[[ ... ]]) do
    if target == v then
        return true
end
return false
  7. Same loop problem exists in findTarget.
  8. You don’t need spawn to call admin.run. It just sets up handlers, it’s not an infinite loop.
  9. You should try to factor out more of your command-specific code. Your Chatted handler is going to be a mess as you add more commands. I would personally structure it something like this:
    local function SplitString(str)
    -- split a string into tokens based on whitespace
    -- see https://stackoverflow.com/questions/1426954/split-string-in-lua
end

--[[ Specific command handlers ]]
local function HandleRespawnCommand(args)
    -- do respawn logic on command args
end

-- map command words to command functions
local function CommandMap = {
    respawn = HandleRespawnCommand
}

-- Call command-specific functions
local function HandleCommand(command, args)
    local func = CommandMap[command]
    
    if (func) then func(args) end
end

-- Just figure out if its a command or not
local function OnChatted(player, msg)
    if (msg:find(prefix) == 1) then
        local tokens = SplitString(msg)
        local cmd = tokens[0]:sub(2)
        local args = {select(2, unpack(tokens))}
        HandleCommand(cmd, args)
    end
end
3 Likes