Admin Commands - Code Review

Help and Feedback Code Review
#1

So I recently have scripted some basic commands for the script more to come soon, I’m looking for any suggestions or ideas to add/remove or change.

DOWNLOAD THE FULL SOURCE: https://www.roblox.com/library/3830389681/Click-Me
PLEASE GIVE ME CREDIT

Module:

local module = {}
 function module.Admins()
	local adminlist = {1,12,744482898} -- UserID List of players that have admin
        return adminlist
end
function module.Bans()
	local banlist = {744482898} -- UserID List of players want banned
        return banlist
end
function module.BanMsg()
 local banmsg = "You have been banned" -- Custom ban message
 return banmsg
end
function module.Music()
	local music = {1234567,098765} -- Music List
        return music
end
return module

Pictures of Private Module
image

image

The basic programming

local module = require(script.Parent.Settings)
local AdminList = module.Admins()
local BanList = module.Bans()
local BanReason = module.BanMsg()
local MusicList = module.Music()
local BanAppeal = " | Removed"
local IsAdmin = false
local IsBanned = false

--Admin?
game.Players.PlayerAdded:connect(function(player)
for _, v in pairs(AdminList) do
if player.UserId == v  then
	script.Parent.Admin.Cmds.LocalPlayer.Value = player.Name
	local Admin = require(script.Parent.Admin.Cmds.Admin)
			Admin.Admin()
IsAdmin = true
print("RS ADMIN | IsAdmin() = true")
end
end
end)
--Banned?
game.Players.PlayerAdded:connect(function(player)
for _, v in pairs(BanList) do
if player.UserId == v then
	player:Kick(BanReason)
	
IsBanned = true
print("RS ADMIN | IsBanned() = true")
end
end
end)





--LocalPlayer
--[[
game.Players.PlayerAdded:connect(function(plr)
	local gui = script.Parent.Admin.Cmds:WaitForChild("LocalPlayer")
	gui.Value = plr.Name
end)]]


local Commands = script.Parent.Admin.Cmds

--Noclip
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":noclip" then
			for _, v in pairs(AdminList) do
			if plr.UserId == v  then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local Noclip = require(script.Parent.Admin.Cmds.Noclip)
			Noclip.Noclip()
			
			   end
			end
		end
	end)
end)
--Fly
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":fly" then
			for _, v in pairs(AdminList) do	
			if plr.UserId == v then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local Fly = require(script.Parent.Admin.Cmds.Fly)
			Fly.Fly()
			
			   end
			end
		end
	end)
end)
--Music
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":music" then
			for _, v in pairs(AdminList) do
			if plr.UserId == v  then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local music = require(script.Parent.Admin.Cmds.Music)
			music.Music()
			
			   end
			end
		end
	end)
end)
--Music Stop
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":stop" then
			for _, v in pairs(AdminList) do
			if plr.UserId == v  then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local musicend = require(script.Parent.Admin.Cmds.MusicStop)
			musicend.StopMusic()
			
			   end
			end
		end
	end)
end)
--Kick
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":kick" then
			for _, v in pairs(AdminList) do
				
			if plr.UserId == v  then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local kick = require(script.Parent.Admin.Cmds.Kick)
			kick.Kick()
			
			   end
			end
		end
	end)
end)
--Cmds
game.Players.PlayerAdded:Connect(function(plr)
	local prefix = module.Prefix()
	plr.Chatted:connect(function(msg)
		if msg == ":cmds" then
			for _, v in pairs(AdminList) do
			
			if plr.UserId == v then
				script.Parent.Admin.Cmds.LocalPlayer.Value = plr.Name
				IsAdmin=true
			local CMD = require(script.Parent.Admin.Cmds.Commands)
			CMD.Commands()
			   end
			end
		end
	end)
end)



--userid 744482898 --name im_sams --rank head dev
6 Likes
#2

It’s preferable that you keep constant values directly in a variable rather than storing in a function to return a copy each time.

module.Admins = {1, 12, etc} over function module.Admins()...

5 Likes
CODE REVIEW - Admin Commands
#3

For my current project, most of my modules interact through each other through functions. Not only does the function call itself only take about 20 nanoseconds, but you’re also able to do other stuff whenever the action is done (i.e. logging to the console). In fact, you find this practice highly recommended in Java.

#4

This is not Java, and Java practices actually tend to be pretty bad.
It is preferrable to access data in Lua directly rather than through function call, both for simplicity in code, less function call hell, and easy differentiation between what is data and what is logic.

If you wish to add extra features (such as logging), a middle man metatable can be used to achieve the same result.

4 Likes
#5

This is good system. But better use commands in table. To make code less with events.

local cmds = {
   print = function(cmd)
     print(cmd)
   end
}

cmds.print("Hello, World!")

Also you may call by separating all arguments in the message

for Segment in string.gmatch(Message, "%S+") do
	table.insert(arguments,Segment)
end
2 Likes