gmodWare / gware-utilities

gmodWare roleplay utilities
2 stars 1 forks source link

Chat commands are horrid #23

Closed StarLight-Oliver closed 1 year ago

StarLight-Oliver commented 1 year ago

This one is actually annoying. As I have already complained about this.

You have a many-to-many check for commands. Which is just insanely bad. Every chat message that is sent is passed through EVERY command you have registered. This is a really BAD Big O notation.

Register your commands into a table.

Then run chat through my string.isCommand function you have included (but not used) and then see if the command name is one of your registered commands, by indexing it.

You already use async registers for your language phases, so no reason to not do that if you were concerned about the race conditions of a gUtils.Command.Register not being registered yet.

StarLight-Oliver commented 1 year ago

The example I have previously shown in discord. Use something like gUtils.Command.Register to add to the cmds table.

local function IsCmd(txt)

    if not (txt[1] == "!" or txt[1] == "/") then return false end

    local spacePos = txt:find(" ")

    local cmdName
    local msg 
    if not spacePos then
        msg = ""
        cmdName = txt:sub(2)
    else 
        cmdName = txt:sub(2, spacePos)
        msg = txt:sub(spacePos + 1)
    end

    return cmdName:lower(), msg
end

local cmds = {
    ["advert"] = -- your command struct / function
}

hook.Add("PlayerSay", "RunningMyCustomCommands", function(ply, txt)

    local name, msg = IsCmd(txt)

    if not name then return end
    if not cmds[name] then return end

    cmd[name] -- Do your thing

    return ""
end)
StarLight-Oliver commented 1 year ago

Adding on to the text-based ones.

It is recommended to send the name and team color, not the player. The team colour is also weirdly darkrp based? team colour is stored on the actual team. https://wiki.facepunch.com/gmod/GM:GetTeamColor or https://wiki.facepunch.com/gmod/team.GetColor are better options to make it less darkrp based

It is slower but it prevents issues where the player doesn't exist on a client and their name appears "weird".

StarLight-Oliver commented 1 year ago

getRollColour could be better if it was a binary left search but due to its size probably doesn't matter. though = feels a bit unneeded as well

the text manipulation for roll seems really messy, you have two formatted %s, %d? could you not just loop over the string pushing them into the text, and when you find the % check where you are, move up one index insert the colour change and the text.

Icarussakka commented 1 year ago

implemented new command handler