Is this a good or bad idea for the admin commands of the past? And does it hold up?

Just a fun-little(ish) question I wanted to ask. :P

I took a look at Person299's Admin Commands script again, and looked at line 229 of the script: it was very interesting seeing how the first take-off admin commands blew up, especially when it came to lines 328-385.

The codes revolve around, for the code presented below, iterating through the string, around commas, checks if a player's name matches, then if the variable number-holder gets 1 player, it'll add the player to "players" (the variable-table-thing in the code); otherwise, it doesn't: the rest is pretty self-explanatory (imo).

However, are these considered good or bad written code? B/c they both, essentially, do:

...
local players = {} -- Code has been rewritten to look & be read much clearer
local onePlayer = nil
local playersWithMatchingNamesFound = 0
...
for i = 1, #commas do  -- You can see this in line 349 & on in the admin; although, if you want Person299's of how to retrieve the positions around commas, it's lines 328-347
	if player.Name == message:sub(commas[i]) then -- Doesn't look like this in the admin
		onePlayer = player
		playersWithMatchingNamesFound = playersWithMatchingNamesFound + 1
	end
end
...
table.insert(players, player)
...
if playersWithMatchingNamesFound == 1 then
	table.insert(players, onePlayer)
end
...
if #players ~= 0 then
	return players
else
	return 0
end
...

-- Later on in the code, which doesn't follow the above code
if playersWithMatchingNamesFound == 1 then
	return {onePlayer}
elseif playersWithMatchingNamesFound > 1 then
	return 0
elseif playersWithMatchingNamesFound == 0 then
	return 0
end

But that's just a nutshell & clearer understanding of what goes on. (The official admin by Person299 is a cluttered, indented-less, elder-style type of code.)

Is this type of code a good idea, or a bad idea? Back then, I knew there were limitations & something like this had to be used, but in the modern day of lua, what is this considered? And does it hold up?

Well, personally I'm going to have to say this is an "eh" type of script. More modern admin commands include the following:

• Tablets that rotate around the LocalPlayer • FE Compatibility • More commands • Cleaner code and more efficient code • Loading scripts (not using loadstring, which is ineffective, but using module scripts) • Command extensions • Trello linking

These are only some of the things more modern admin commands include that Person299's Admin Commands don't. Those admin commands were definitely used quite a lot, and some games still use them although personally I wouldn't go about making commands like he did. I've made two different versions of admin commands and they didn't quite come out like his. They were cleaner and were easy to understand.

When creating admin commands you need to consider the following:

• What commands do you want to include in your admin commands? • Will your admin commands use some sort of GUI or Tablets to function? • Are you going to make these commands FE compatible? <-- Always say yes to this question. • Will command extensions be available to ease the struggle of your users who want to make custom commands? • Will you have a defined or custom prefix?

These questions are important to ask yourself before creating admin commands. I would go with this type of approach when making admin commands:

re = game.ReplicatedStorage.NetworkFolder.RE
prefix = "="
game.Players.PlayerAdded:connect(function(plr)
	plr.Chatted:connect(function(msg)
		if string.sub(string.lower(msg),1,1) == prefix then
			if string.sub(string.lower(msg), 2, 5) == "kill" then
				re:FireServer("kill", msg)
			end
		end
	end)
end)

This is code is effective and allows for you to add commands easily (obviously you'd use a loader so users would add commands through an external script).

I hope I helped you out. :)

External Script Command Adding:

-- This is the basic command script. The New Commands table would look like this:

table = {
	newCommand = {
		2, -- Minimum
		6, -- Maximum
		"reset" -- Command name
	}
}

Regular commands script:

re = game.ReplicatedStorage.NetworkFolder.RE
prefix = "="
newCommand = require(script.Parent.ExtraCommands)
newCommandsTable = newCommand.getCommands()
game.Players.PlayerAdded:connect(function(plr)
	plr.Chatted:connect(function(msg)
		if string.sub(string.lower(msg),1,1) == prefix then
			if string.sub(string.lower(msg), 2, 5) == "kill" then
				re:FireServer("kill", msg)
			elseif string.sub(string.lower(msg), newCommandsTable[1][1], newCommandsTable[1][2]) == newCommandsTable[1][3] then
				re:FireServer(newCommandersTable[1][3], msg)
			end
		end
	end)
end)

Inside of ExtraCommands script (It's a module script):

local m = {}

m.getCommands = function()

	-- You can create new commands below here.
	-- 1) Edit the maximum value (leave minimum the same)
	-- 2) Change the command name
	-- 3) I haven't implemented a programming feature so :/
	
	coms = {
		newCommand = {
			2, -- Minimum
			6, -- Maximum
			"reset" -- Command name
		}
	}
	
	return coms

end

return m