-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/role preference options #1142
Changes from 2 commits
5e549e3
9ff6bab
fdd18d7
9121aa3
0cad328
bc466e5
5de648e
845348c
2c72f38
393f6f3
24cc875
988ea27
3b3b866
f718dc8
c3d9fde
af3e270
ea96711
6292d93
15b40ce
43d8d3d
9a1b12b
3d2274a
8e9f04f
e97f203
dfd5605
a98d02d
5af37d2
dae87eb
aaa1f19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ roleselection.finalRoles = {} | |
roleselection.selectableRoles = nil | ||
roleselection.baseroleLayers = {} | ||
roleselection.subroleLayers = {} | ||
roleselection.trickleDownRate = 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still can have a sane default, as it was there before too with the random(3) == 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still 0, should be at least 3. |
||
|
||
-- Convars | ||
roleselection.cv = { | ||
|
@@ -33,7 +34,7 @@ roleselection.cv = { | |
|
||
--- | ||
-- @realm server | ||
ttt_max_baseroles_pct = CreateConVar("ttt_max_baseroles_pct", "0", {FCVAR_NOTIFY, FCVAR_ARCHIVE}, "Maximum amount of different baseroles based on player amount. ttt_max_baseroles needs to be 0") | ||
ttt_max_baseroles_pct = CreateConVar("ttt_max_baseroles_pct", "0", {FCVAR_NOTIFY, FCVAR_ARCHIVE}, "Maximum amount of different baseroles based on player amount. ttt_max_baseroles needs to be 0"), | ||
} | ||
|
||
-- saving and loading | ||
|
@@ -572,10 +573,14 @@ local function SetSubRoles(plys, availableRoles, selectableRoles, selectedForced | |
local plysAmount = #plys | ||
local availableRolesAmount = #availableRoles | ||
local tmpSelectableRoles = table.Copy(selectableRoles) | ||
-- plys are already shuffled, continue with existing priority | ||
local newPlys = table.Copy(plys) | ||
|
||
while plysAmount > 0 and availableRolesAmount > 0 do | ||
local pick = math.random(plysAmount) | ||
local ply = plys[pick] | ||
for i = #newPlys, 1, -1 do | ||
Comment on lines
-576
to
+579
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The for loop here also has the wrong behaviour. It only goes through every player once. But it could be that some players are not assigned. |
||
if not (plysAmount > 0 and availableRolesAmount > 0) then | ||
break | ||
end | ||
local ply = plys[i] | ||
|
||
local rolePick = math.random(availableRolesAmount) | ||
local subrole = availableRoles[rolePick] | ||
|
@@ -586,16 +591,8 @@ local function SetSubRoles(plys, availableRoles, selectableRoles, selectedForced | |
roleCount = roleCount - selectedForcedRoles[subrole] | ||
end | ||
|
||
local minKarmaCVar = GetConVar("ttt_" .. roleData.name .. "_karma_min") | ||
local minKarma = minKarmaCVar and minKarmaCVar:GetInt() or 0 | ||
|
||
-- give this player the role if | ||
if plysAmount <= roleCount -- or there aren't enough players anymore to have a greater role variety | ||
or ply:GetBaseKarma() > minKarma -- or the player has enough karma | ||
and not ply:GetAvoidRole(subrole) -- and the player doesn't avoid this role | ||
or math.random(3) == 2 -- or if the randomness decides | ||
then | ||
table.remove(plys, pick) | ||
if ply:CanSelectRole(roleData, plysAmount, availableRolesAmount) then | ||
table.remove(newPlys, i) | ||
|
||
roleselection.finalRoles[ply] = subrole | ||
|
||
|
@@ -782,30 +779,22 @@ end | |
local function SelectBaseRolePlayers(plys, subrole, roleAmount) | ||
local curRoles = 0 | ||
local plysList = {} | ||
local roleData = roles.GetByIndex(subrole) | ||
|
||
local minKarmaCVar = GetConVar("ttt_" .. roles.GetByIndex(subrole).name .. "_karma_min") | ||
local min_karmas = minKarmaCVar and minKarmaCVar:GetInt() or 0 | ||
|
||
while curRoles < roleAmount and #plys > 0 do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesnt removing this while loop here means, that players might not get a role in this pass? As far as I can see, the "CanSelectRole" method can most often just return false. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also unsure about what this change implies for the selection process further down. We need to think of the assumptions, that were made for what this function does codewise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (After requested Review) |
||
-- select random index in plys table | ||
local pick = math.random(#plys) | ||
|
||
for i = #plys, 1, -1 do | ||
-- the player we consider | ||
local ply = plys[pick] | ||
|
||
-- give this player the role if | ||
if subrole == ROLE_INNOCENT -- this role is an innocent role | ||
or #plys <= roleAmount -- or there aren't enough players anymore to have a greater role variety | ||
or ply:GetBaseKarma() > min_karmas -- or the player has enough karma | ||
and not ply:GetAvoidRole(subrole) -- and the player doesn't avoid this role | ||
or math.random(3) == 2 -- or if the randomness decides | ||
then | ||
table.remove(plys, pick) | ||
local ply = plys[i] | ||
-- do not unbrace this | ||
if not (curRoles < roleAmount and #plys > 0) then break end | ||
EntranceJew marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if subrole == ROLE_INNOCENT or ply:CanSelectRole(roleData, #plys, roleAmount) then | ||
curRoles = curRoles + 1 | ||
plysList[curRoles] = ply | ||
|
||
roleselection.finalRoles[ply] = subrole -- give the player the final baserole (maybe he will receive his subrole later) | ||
-- give the player the final baserole (maybe he will receive his subrole later) | ||
roleselection.finalRoles[ply] = subrole | ||
|
||
table.remove(plys, i) | ||
end | ||
end | ||
|
||
|
@@ -826,6 +815,9 @@ function roleselection.SelectRoles(plys, maxPlys) | |
|
||
plys = roleselection.GetSelectablePlayers(plys or player.GetAll()) | ||
|
||
-- Randomize role assignment by shuffling the list early. | ||
table.Shuffle(plys) | ||
|
||
maxPlys = maxPlys or #plys | ||
|
||
if maxPlys < 2 then return end -- we don't need to select anything if there is just one player | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also in my opinion there are no RNG improvements, as you said in this PR, it generally only "made dead code not dead"