Role-selection bug: Forced roles are not deducted from the available roles amount list

[Alf21]

e.g. here https://github.com/TTT-2/TTT2/blob/cee9c123983d7ba9e666f1d666ce5fd987663a12/gamemodes/terrortown/gamemode/server/sv_roleselection.lua#L496, a decrease of the roles' amount is missing, as result, the function is not working properly (the roles' amount limit is exceeded)

[ZenBre4ker]

To solve this issue you only need to decrease the roleCount of selectableRoles in the function SelectForcedRoles().

One idea would be to do it like this:

local function SelectForcedRoles(plys, selectableRoles)
    --[...]
    for subrole, forcedPlys in pairs(transformed) do
        --[...]
        for i = 1, amount do
                        --[...]
            if roleCount <= curCount then break end -- if the limit is reached, stop selection for this role
                        --[...]
        end

        --NEW CODE
        if roleCount <=curCount then
                table.remove(selectableRoles, rd)
        else
            roleCount = roleCount-curCount
                selectableRoles[rd]=roleCount
            end
        --END
    end

    --NEW CODE
    roleSelection.selectableRoles = selectableRoles --Also update selectableRoles
        --END

    roleselection.forcedRoles = {}

    return selectedPlys
end 

This is not perfect as you could instead just decrement roleCount on the go instead of incrementing curCount and update it in the end, but it should be the solution to the problem and work fine with the rest of the code.

Don't know yet how Github works otherwise I'd try to commit it and make a pullrequest.

[Alf21]

The problem is that modifying the original table could lead to some unexpected behavior (if you wanna use this list for later selections / in-round selections as well). That's why I thought about decreasing it "manually" (if calling a function).

Anyways, thanks for your input, always appreciated. Yea feel free to try new things here, probably you/we can learn some new things. 👍

[ZenBre4ker]

Okay, you are right and I just had to go further down the code to find a problem which could give a nil value.....

Well, how about copying selectableRoles and changing that to selectedForcedRoles, which counts the already assigned roles, so that it is only used later for the calculation of amount in SelectBaseRolePlayers() My idea would look like this: SelectForcedRoles only needs a third parameter and assigns at the end the curCount to the new table

local function SelectForcedRoles(plys, selectableRoles, selectedForcedRoles)
        --[...]
    for subrole, forcedPlys in pairs(transformed) do
        --[...]
        for i = 1, amount do
              --[...]
        end
                --NEW Code
        selectedForcedRoles[rd] = curCount
                --End
    end
        --[...]
end

In the roleselection.SelectRoles we would create the copy and use it to calculate the amount of roles

function roleselection.SelectRoles(plys, maxPlys)
    --[...]
    local selectableRoles = roleselection.GetSelectableRolesList(maxPlys, allAvailableRoles) -- update roleselection.selectableRoles table
    local selectedForcedRoles = table.Copy(selectableRoles)

    -- Select forced roles at first
    local selectedForcedPlys = SelectForcedRoles(plys, selectableRoles, selectedForcedRoles) -- this updates roleselection.finalRoles table and returns a key based list of selected players
    --[...]
    if #plysFirstPass > 0 then
        --[...]
        for i = 1, #list do
            --[...]
            local amount = selectableRoles[roleData] - selectedForcedRoles[roleData]
            local baseRolePlys = SelectBaseRolePlayers(plysFirstPass, roleData, amount)
            --[...]
        end
        --[...]
    end
    --[...]
end

In general this only changes something at that one place, where we need to know how many roles are available, it generally should work for innocents and traitor Roles too and the only overhead is the copy of the table, which only needs to iterate over all roles. Therefore is playernumber independent.

[Alf21]

Okay the layering PR got merged, now we can discuss the current problem with forcing roles. :)

Your suggestions should work, better said it also is the first idea I would try fixing the bug.

[ZenBre4ker]

One issue I also noticed and saw in the code, is that when you force more roles than available, it stops assigning them. This may be a wanted feature, otherwise I also might suggest to make a convar to let the Server owner decide, whether to overassign roles or not, because forcing, well is still overwriting normal stuff.

[Alf21]

Regarding old behaviors, the default functionality should be the following:

• Forcing a role should set your role no matter how much other roles exists. (DONE)
• ForceNextround should force a role if available (not exceeding the limit)
• HardforceNextround should force a role (no matter how much other roles exists)