From c840bb91b1f8e7d7da1349adf63658f1081a8e38 Mon Sep 17 00:00:00 2001 From: Doubleumc Date: Fri, 19 Jul 2024 16:25:07 -0400 Subject: [PATCH] pick_weight over pickweight (#6760) # About the pull request Removes `pickweight` and switches over to `pick_weight`. The only difference is that `pickweight` treated weight 0 as weight 1, and `pick_weight` does not. Only `utility_closets` and `flow` had 0-weight entries, and I corrected those. Updates `pick_weight` to TG's version, which fixes a statistical error: https://github.com/tgstation/tgstation/issues/71271 https://github.com/tgstation/tgstation/pull/71273 # Explain why it's good for the game Less redundant code. More statistical accuracy. # Testing Photographs and Procedure Boots. # Changelog :cl: code: Fixed and refactored probability weighting for pick_weight /:cl: --- code/__HELPERS/_lists.dm | 18 ++++++++++++------ code/__HELPERS/lists.dm | 16 ---------------- code/game/machinery/computer/arcade.dm | 4 ++-- .../crates_lockers/closets/utility_closets.dm | 2 +- code/game/supplyshuttle.dm | 2 +- code/modules/nightmare/nmnodes/flow.dm | 5 ++--- 6 files changed, 18 insertions(+), 29 deletions(-) diff --git a/code/__HELPERS/_lists.dm b/code/__HELPERS/_lists.dm index e46c92df543a..aa73d6008e02 100644 --- a/code/__HELPERS/_lists.dm +++ b/code/__HELPERS/_lists.dm @@ -128,17 +128,23 @@ * You should only pass integers in. */ /proc/pick_weight(list/list_to_pick) + if(length(list_to_pick) == 0) + return null + var/total = 0 - var/item - for(item in list_to_pick) + for(var/item in list_to_pick) if(!list_to_pick[item]) list_to_pick[item] = 0 total += list_to_pick[item] - total = rand(0, total) - for(item in list_to_pick) - total -= list_to_pick[item] - if(total <= 0 && list_to_pick[item]) + total = rand(1, total) + for(var/item in list_to_pick) + var/item_weight = list_to_pick[item] + if(item_weight == 0) + continue + + total -= item_weight + if(total <= 0) return item return null diff --git a/code/__HELPERS/lists.dm b/code/__HELPERS/lists.dm index d5212611a04b..32ea0f5ec32e 100644 --- a/code/__HELPERS/lists.dm +++ b/code/__HELPERS/lists.dm @@ -87,22 +87,6 @@ result = first ^ second return result -//Pretends to pick an element based on its weight but really just seems to pick a random element. -/proc/pickweight(list/L) - var/total = 0 - var/item - for (item in L) - if (!L[item]) - L[item] = 1 - total += L[item] - - total = rand(1, total) - for (item in L) - total -=L [item] - if (total <= 0) - return item - return null - /// Pick a random element from the list and remove it from the list. /proc/pick_n_take(list/L) RETURN_TYPE(L[_].type) diff --git a/code/game/machinery/computer/arcade.dm b/code/game/machinery/computer/arcade.dm index ff8f3959d64e..4f6f4df4ef08 100644 --- a/code/game/machinery/computer/arcade.dm +++ b/code/game/machinery/computer/arcade.dm @@ -123,7 +123,7 @@ src.temp = "[src.enemy_name] has fallen! Rejoice!" if(!length(contents)) - var/prizeselect = pickweight(prizes) + var/prizeselect = pick_weight(prizes) new prizeselect(src.loc) if(istype(prizeselect, /obj/item/toy/gun)) //Ammo comes with the gun @@ -176,5 +176,5 @@ if(2) num_of_prizes = rand(0,2) for(num_of_prizes; num_of_prizes > 0; num_of_prizes--) - empprize = pickweight(prizes) + empprize = pick_weight(prizes) new empprize(src.loc) diff --git a/code/game/objects/structures/crates_lockers/closets/utility_closets.dm b/code/game/objects/structures/crates_lockers/closets/utility_closets.dm index b000fd5733a2..0bf39322d107 100644 --- a/code/game/objects/structures/crates_lockers/closets/utility_closets.dm +++ b/code/game/objects/structures/crates_lockers/closets/utility_closets.dm @@ -23,7 +23,7 @@ . = ..() #ifndef UNIT_TESTS - switch (pickweight(list("small" = 55, "aid" = 25, "tank" = 10, "both" = 10, "nothing" = 0, "delete" = 0))) + switch (pick_weight(list("small" = 55, "aid" = 25, "tank" = 10, "both" = 10, "nothing" = 1, "delete" = 1))) #else var/test = "both" switch (test) // We don't want randomness in tests diff --git a/code/game/supplyshuttle.dm b/code/game/supplyshuttle.dm index 53b64cbedfad..8974eb36187f 100644 --- a/code/game/supplyshuttle.dm +++ b/code/game/supplyshuttle.dm @@ -532,7 +532,7 @@ GLOBAL_DATUM_INIT(supply_controller, /datum/controller/supply, new()) for(var/datum/supply_packs_asrs/crate in cratelist) var/weight = (floor(10000/crate.cost)) weighted_crate_list[crate] = weight - return pickweight(weighted_crate_list) + return pick_weight(weighted_crate_list) //To stop things being sent to centcomm which should not be sent to centcomm. Recursively checks for these types. /datum/controller/supply/proc/forbidden_atoms_check(atom/A) diff --git a/code/modules/nightmare/nmnodes/flow.dm b/code/modules/nightmare/nmnodes/flow.dm index 1ca8c48f88b8..5f94326bbcaa 100644 --- a/code/modules/nightmare/nmnodes/flow.dm +++ b/code/modules/nightmare/nmnodes/flow.dm @@ -67,15 +67,14 @@ if(!.) return var/list/datum/nmnode/pickables = choices.Copy() for(var/datum/nmnode/node as anything in pickables) - if(isnum(node.raw["weight"])) - pickables[node] = node.raw["weight"] + pickables[node] = isnum(node.raw["weight"]) ? node.raw["weight"] : 1 var/list/datum/nmnode/picked = list() var/remaining = src.amount #if defined(UNIT_TESTS) remaining = length(pickables) // Force all to be picked for testing (this could potentially make false positives though) #endif while(length(pickables) && remaining > 0) - var/datum/nmnode/node = pickweight(pickables) + var/datum/nmnode/node = pick_weight(pickables) remaining-- pickables -= node picked += node