Feedback on Command Handler

Help and Feedback Code Review
, ,
#1
  • The code is supposed to take in a message sent by an Admin and handle it and run the specified command and give it the parts of the message as well as the sender of the message.
  • When it comes to improving the code I’m not really sure what could improve other than the string handling but I’m unsure of how.
  • I’d like to know if this code would be seen as well optimized and efficient and/or if there is something I should think about doing in the future and/or now.

To save runtime I added a spawn() at the System.addToLogs in parseMsg. This way the command runs faster and the log is saved at the same time. Before this, it was just the function without the spawn.

---------------
-- Functions --
---------------
local function checkIfAlias(CommandName)
	for _,v in pairs(cmds) do
		if v.Name ~= "AdminList" and v.Aliases ~= nil then
			for _,k in pairs(v.Aliases) do
				if string.lower(k) == CommandName then
					return v.Name
				end
			end
		end
	end
	return nil
end

local function parseMsg(plr, msg)
	msg = string.lower(msg)
	local PrefixMatch = string.match(msg,"^"..Prefix)
	if PrefixMatch then
		
		msg = string.gsub(msg,PrefixMatch,"")
		local Arguments = {}
		
		for Argument in string.gmatch(msg, "[^%s]+") do
			table.insert(Arguments, Argument)
		end
		local CommandName = Arguments[1]
		table.remove(Arguments, 1)
		local Alias = checkIfAlias(CommandName)
		if Alias ~= nil then
			CommandName = Alias
		end
		local CommandFunc = cmds[CommandName].Run
		if CommandFunc ~= nil then
			spawn(function()
				System.addToLogs(msg, plr)
			end)
			CommandFunc(plr, Arguments)
		end
	end
end

local function isAdmin(plr)
	for _,Admin in pairs(cmds.Admins:GetAsync("Admins")) do
		if type(Admin) == "number" and Admin == plr.UserId then
			return true
		end
	end
	return false
end

------------
-- Events --
------------

Players.PlayerAdded:Connect(function(plr)
	print(0)
	plr.Chatted:Connect(function(msg, r)
		if not r and isAdmin(plr) then 
			parseMsg(plr, msg)
		end
	end)
end)

An example of the command table in module script:

cmds.commandName  = {
   Name = "commandName",
   Desc = "commandDescription",
   Use = "commadName [args]",
   Group = "Owner only/Admin",
   Aliases = {"list of aliases, if any"},
   Run = function(sender, args) 
        -- Code goes here
   		end
}
2 Likes
#2

Why not use coroutine (I may have spelled it wrong)? spawn has some delay, so there might be some difference in speed with those two.

1 Like
#3

Well, when I first tried using coroutine it didn’t launch the function. I assumed this was because it had to be resumed or something but I thought that it would be less efficient then. I’ll look into the coroutine function. Thanks

#4

You should thinking of replacing spawn() with coroutines, coroutines have a lot more than spawn() and are faster.

Another thing would be to indent your code and the consistency of the variables. Also take in count of the code readability.

Here’s how I would write your code.

local function checkPlayerAlias(CommandName)
	for _,v in pairs(cmds) do
		if v.Name ~= "AdminList" and v.Aliases ~= nil then
			for _, k in pairs(v.Aliases) do
				if string.lower(k) == CommandName then
					return v.Name
				end
			end
		end
	end
	return nil
end

local function parseMessage(player, message)
	local PrefixMatch = string.match(message, "^"  .. Prefix)
	
	message = string.lower(message)
	
	if PrefixMatch then
		
		local Arguments = {}
		
		message = string.gsub(message, PrefixMatch, "")
		
		for argument in string.gmatch(message, "[^%s]+") do
			table.insert(Arguments, argument)
		end
		
		local CommandName = Arguments[1]
		local Alias = checkPlayerAlias(CommandName)
		
		table.remove(Arguments, 1)
		
		if Alias ~= nil then
			CommandName = Alias
		end
		
		local CommandFunc = cmds[CommandName].Run
		
		if CommandFunc ~= nil then
			
			local thread = coroutine.create(function()
				System.addToLogs(message, player)
			end)
			
			coroutine.wrap(thread)
			
			CommandFunc(player, Arguments)
		end
	end
end

local function isPlayerAdmin(player)
	for _, Admin in pairs(cmds.Admins:GetAsync("Admins")) do
		if typeof(Admin) == "number" and Admin == player.UserId then
			return true
		end
	end
	return false
end

------------
-- Events --
------------

Players.PlayerAdded:Connect(function(player)
	player.Chatted:Connect(function(message, processed)
		if not processed and isPlayerAdmin(player) then 
			parseMessage(player, message)
		end
	end)
end)

cmds.commandName  = {
   Name = "commandName",
   Desc = "commandDescription",
   Use = "commadName [args]",
   Group = "Owner only/Admin",
   Aliases = {"list of aliases, if any"},

     Run = function(sender, args) 

     end
}
1 Like
#5

Alright, I’ll think about that, for the future, thank you!