Since this is for already working code I will use this category.
it says to include rblx file: Place1.rbxl (52.4 KB)
here is my code
local command = "/admin"
game.Players.PlayerAdded:Connect(function(player)
player.Chatted:Connect(function(msg)
local lowered = msg:lower()
if string.find(lowered, command) then
local arg = string.split(lowered, " ")
if arg[2] == "shop" then
print(arg[2])
end
end
end)
end)
I was wondering if there is a way to do this better than what I have already done?
local command = "/admin"
local presets = {}
function presets.shop(arg)
end
game.Players.PlayerAdded:Connect(function(player)
player.Chatted:Connect(function(msg)
local lowered = msg:lower()
if string.find(lowered, command) then
local arg = string.split(lowered, " ")
if presets[arg[2]] then presets[arg[2]](arg) end
end
end)
end)
Utilizing tables and adding functions into them would help you futureproof any other issues you might come across and saving you time such as not having to adjust conditions when removing an argument. Easier to focus and insert commands due to its clean separation.
local lowered = msg:lower() should be string.lower(msg)
the prefix is also incredibly bad and inconvenient (use : or ; or something) and will work even if /admin is not the first argument and some other string is. for example, the player says "t kill me /admin would work if you had a kill me command.
game.Players should be Players and Players should be game:GetService('Players'). this is good practise.
game.Players.PlayerAdded is fine, but what if the code before the game.Players.PlayerAdded is delayed, and the player is already in the game? then PlayerAdded wont fire for that player. you can fix this by looping through all the players and calling the function playeradded uses on them
additionally, like nvthane said, you can use tables for each command
local commands = {}
local prefix = ';'
local independentPrefix = false -- if false, then you'll have to type commands like ;kill or /adminkill. if true, then you have to type it like ; kill or /admin kill
commands.kill = {minimumParameterCount = 1, execute = function(player)
-- 'player' is lowercase. therefore if you want to search for any player, compare the 'player' argument (string) with their lowercase name/display name
if (player == 'all') then
-- ...
end
end}
local function playerAdded(player)
player.Chatted:Connect(function(msg)
local parameters = string.split(string.lower(msg), ' ') -- lowers the message and splits it
local firstParam = parameters[1]
local secondParam = parameters[2]
if (not firstParam) then return end
if (string.sub(firstParam, 1, #prefix) ~= prefix) then return end
local paramCount;
local targetCommand, parameterStart;
if (independentPrefix) then
if (not secondParam) then return end
paramCount = #parameters - 2 -- subtract the prefix and command name from the count
targetCommand = secondParam -- the second param should be
else
paramCount = #parameters - 1 -- subtract the first string.split, the prefix and command together, from the count
targetCommand = string.sub(firstParam, #prefix + 1, #firstParam) -- get everything after the prefix in the same word
end
local command = commands[targetCommand]
if (command and paramCount >= command.minimumParameterCount) then -- if the command exists and we have at least the minimum amount of parameters
command.execute(select(independentPrefix and (3) or (2), table.unpack(parameters))) -- we select the 3rd parameter and everything after that, and put it into the command.execute function
end
end)
end
for _, v in ipairs(game.Players:GetPlayers()) do playerAdded(v) end
game.Players.PlayerAdded:Connect(playerAdded)
thanks,
I will replace some of the script, I didnt know about game:Getservice(“players”) being better practice, Also: why is string.lower(string) better than msg:lower() the method applies self to the argument.