From bd1098b87409ea5509e43c8db1d8966ddabac0e2 Mon Sep 17 00:00:00 2001 From: AnturK Date: Fri, 16 Jun 2023 03:58:58 +0200 Subject: [PATCH] Removes amount_list_postion from reagent containers, adds related unit test. (#76057) We had more issues like what #76013 addressed, now they're gone. Variable transfer amount is now explicit. Amount is now inferred from current value, performance concern here is minimal. Less work and mistakes when making new types. --- .../food_and_drinks/machinery/stove.dm | 1 - code/modules/reagents/reagent_containers.dm | 29 ++++++++++--------- .../reagents/reagent_containers/chem_pack.dm | 2 +- .../reagents/reagent_containers/cups/_cup.dm | 1 - .../reagent_containers/cups/drinks.dm | 2 +- .../reagents/reagent_containers/dropper.dm | 1 - .../reagents/reagent_containers/hypospray.dm | 1 + .../reagents/reagent_containers/misc.dm | 2 +- .../reagents/reagent_containers/pill.dm | 2 +- .../reagents/reagent_containers/spray.dm | 2 +- .../reagents/reagent_containers/syringes.dm | 3 ++ code/modules/unit_tests/_unit_tests.dm | 1 + .../unit_tests/reagent_container_defaults.dm | 12 ++++++++ 13 files changed, 38 insertions(+), 21 deletions(-) create mode 100644 code/modules/unit_tests/reagent_container_defaults.dm diff --git a/code/modules/food_and_drinks/machinery/stove.dm b/code/modules/food_and_drinks/machinery/stove.dm index 87cf51f7cb8..5d46d60550f 100644 --- a/code/modules/food_and_drinks/machinery/stove.dm +++ b/code/modules/food_and_drinks/machinery/stove.dm @@ -34,7 +34,6 @@ volume = 200 possible_transfer_amounts = list(20, 50, 100, 200) amount_per_transfer_from_this = 50 - amount_list_position = 2 reagent_flags = REFILLABLE | DRAINABLE custom_materials = list(/datum/material/iron =SHEET_MATERIAL_AMOUNT * 2.5) w_class = WEIGHT_CLASS_BULKY diff --git a/code/modules/reagents/reagent_containers.dm b/code/modules/reagents/reagent_containers.dm index 8f3bc31d069..ad7d54a09ed 100644 --- a/code/modules/reagents/reagent_containers.dm +++ b/code/modules/reagents/reagent_containers.dm @@ -4,12 +4,12 @@ icon = 'icons/obj/medical/chemical.dmi' icon_state = null w_class = WEIGHT_CLASS_TINY - /// The maximum amount of reagents per transfer that will be moved out of this reagent container. This value's position in possible_transfer_amounts should be reflected in amount_list_position. + /// The maximum amount of reagents per transfer that will be moved out of this reagent container. var/amount_per_transfer_from_this = 5 + /// Does this container allow changing transfer amounts at all, the container can still have only one possible transfer value in possible_transfer_amounts at some point even if this is true + var/has_variable_transfer_amount = TRUE /// The different possible amounts of reagent to transfer out of the container var/list/possible_transfer_amounts = list(5,10,15,20,25,30) - /// Where we are in the possible transfer amount list. Number should match the position in possible_transfer_amounts corresponding to amount_per_transfer_from_this. - var/amount_list_position = 1 /// The maximum amount of reagents this container can hold var/volume = 30 /// Reagent flags, a few examples being if the container is open or not, if its transparent, if you can inject stuff in and out of the container, and so on @@ -47,15 +47,15 @@ var/datum/disease/F = new spawned_disease() var/list/data = list("viruses"= list(F)) reagents.add_reagent(/datum/reagent/blood, disease_amount, data) - add_initial_reagents() /obj/item/reagent_containers/examine() . = ..() - if(possible_transfer_amounts.len > 1) - . += span_notice("Left-click or right-click in-hand to increase or decrease its transfer amount.") - else if(possible_transfer_amounts.len) - . += span_notice("Left-click or right-click in-hand to view its transfer amount.") + if(has_variable_transfer_amount) + if(possible_transfer_amounts.len > 1) + . += span_notice("Left-click or right-click in-hand to increase or decrease its transfer amount.") + else if(possible_transfer_amounts.len) + . += span_notice("Left-click or right-click in-hand to view its transfer amount.") /obj/item/reagent_containers/create_reagents(max_vol, flags) . = ..() @@ -77,10 +77,12 @@ reagents.add_reagent_list(list_reagents) /obj/item/reagent_containers/attack_self(mob/user) - change_transfer_amount(user, FORWARD) + if(has_variable_transfer_amount) + change_transfer_amount(user, FORWARD) /obj/item/reagent_containers/attack_self_secondary(mob/user) - change_transfer_amount(user, BACKWARD) + if(has_variable_transfer_amount) + change_transfer_amount(user, BACKWARD) /obj/item/reagent_containers/proc/mode_change_message(mob/user) return @@ -89,14 +91,15 @@ var/list_len = length(possible_transfer_amounts) if(!list_len) return + var/index = possible_transfer_amounts.Find(amount_per_transfer_from_this) || 1 switch(direction) if(FORWARD) - amount_list_position = (amount_list_position % list_len) + 1 + index = (index % list_len) + 1 if(BACKWARD) - amount_list_position = (amount_list_position - 1) || list_len + index = (index - 1) || list_len else CRASH("change_transfer_amount() called with invalid direction value") - amount_per_transfer_from_this = possible_transfer_amounts[amount_list_position] + amount_per_transfer_from_this = possible_transfer_amounts[index] balloon_alert(user, "transferring [amount_per_transfer_from_this]u") mode_change_message(user) diff --git a/code/modules/reagents/reagent_containers/chem_pack.dm b/code/modules/reagents/reagent_containers/chem_pack.dm index 7e6df4a0b4a..3345f1e99ef 100644 --- a/code/modules/reagents/reagent_containers/chem_pack.dm +++ b/code/modules/reagents/reagent_containers/chem_pack.dm @@ -10,7 +10,7 @@ resistance_flags = ACID_PROOF var/sealed = FALSE fill_icon_thresholds = list(10, 20, 30, 40, 50, 60, 70, 80, 90, 100) - possible_transfer_amounts = list() + has_variable_transfer_amount = FALSE /obj/item/reagent_containers/chem_pack/AltClick(mob/living/user) if(user.can_perform_action(src, NEED_DEXTERITY) && !sealed) diff --git a/code/modules/reagents/reagent_containers/cups/_cup.dm b/code/modules/reagents/reagent_containers/cups/_cup.dm index 45c2f00528b..8e0b254a2e0 100644 --- a/code/modules/reagents/reagent_containers/cups/_cup.dm +++ b/code/modules/reagents/reagent_containers/cups/_cup.dm @@ -2,7 +2,6 @@ name = "open container" amount_per_transfer_from_this = 10 possible_transfer_amounts = list(5, 10, 15, 20, 25, 30, 50) - amount_list_position = 2 volume = 50 reagent_flags = OPENCONTAINER | DUNKABLE spillable = TRUE diff --git a/code/modules/reagents/reagent_containers/cups/drinks.dm b/code/modules/reagents/reagent_containers/cups/drinks.dm index 2cdf7053aa4..2cf34da1a62 100644 --- a/code/modules/reagents/reagent_containers/cups/drinks.dm +++ b/code/modules/reagents/reagent_containers/cups/drinks.dm @@ -49,7 +49,7 @@ throwforce = 1 amount_per_transfer_from_this = 5 custom_materials = list(/datum/material/iron=SMALL_MATERIAL_AMOUNT) - possible_transfer_amounts = list(5) + has_variable_transfer_amount = FALSE volume = 5 flags_1 = CONDUCT_1 spillable = TRUE diff --git a/code/modules/reagents/reagent_containers/dropper.dm b/code/modules/reagents/reagent_containers/dropper.dm index fd6397ec5f5..2a005d27826 100644 --- a/code/modules/reagents/reagent_containers/dropper.dm +++ b/code/modules/reagents/reagent_containers/dropper.dm @@ -6,7 +6,6 @@ inhand_icon_state = "dropper" worn_icon_state = "pen" amount_per_transfer_from_this = 5 - amount_list_position = 5 possible_transfer_amounts = list(1, 2, 3, 4, 5) volume = 5 reagent_flags = TRANSPARENT diff --git a/code/modules/reagents/reagent_containers/hypospray.dm b/code/modules/reagents/reagent_containers/hypospray.dm index 03791bf9d7a..e1a444b52d3 100644 --- a/code/modules/reagents/reagent_containers/hypospray.dm +++ b/code/modules/reagents/reagent_containers/hypospray.dm @@ -111,6 +111,7 @@ lefthand_file = 'icons/mob/inhands/equipment/medical_lefthand.dmi' righthand_file = 'icons/mob/inhands/equipment/medical_righthand.dmi' amount_per_transfer_from_this = 15 + has_variable_transfer_amount = FALSE volume = 15 ignore_flags = 1 //so you can medipen through spacesuits reagent_flags = DRAWABLE diff --git a/code/modules/reagents/reagent_containers/misc.dm b/code/modules/reagents/reagent_containers/misc.dm index 44e778647db..cae8582392b 100644 --- a/code/modules/reagents/reagent_containers/misc.dm +++ b/code/modules/reagents/reagent_containers/misc.dm @@ -125,7 +125,7 @@ item_flags = NOBLUDGEON reagent_flags = OPENCONTAINER amount_per_transfer_from_this = 5 - possible_transfer_amounts = list() + has_variable_transfer_amount = FALSE volume = 5 spillable = FALSE diff --git a/code/modules/reagents/reagent_containers/pill.dm b/code/modules/reagents/reagent_containers/pill.dm index 72915b66420..ead552c95a2 100644 --- a/code/modules/reagents/reagent_containers/pill.dm +++ b/code/modules/reagents/reagent_containers/pill.dm @@ -7,7 +7,7 @@ worn_icon_state = "nothing" lefthand_file = 'icons/mob/inhands/equipment/medical_lefthand.dmi' righthand_file = 'icons/mob/inhands/equipment/medical_righthand.dmi' - possible_transfer_amounts = list() + has_variable_transfer_amount = FALSE volume = 50 grind_results = list() var/apply_type = INGEST diff --git a/code/modules/reagents/reagent_containers/spray.dm b/code/modules/reagents/reagent_containers/spray.dm index 93e5255f85e..297b7f82273 100644 --- a/code/modules/reagents/reagent_containers/spray.dm +++ b/code/modules/reagents/reagent_containers/spray.dm @@ -235,7 +235,7 @@ lefthand_file = 'icons/mob/inhands/weapons/plants_lefthand.dmi' righthand_file = 'icons/mob/inhands/weapons/plants_righthand.dmi' amount_per_transfer_from_this = 1 - possible_transfer_amounts = list(1) + has_variable_transfer_amount = FALSE can_toggle_range = FALSE current_range = 1 volume = 10 diff --git a/code/modules/reagents/reagent_containers/syringes.dm b/code/modules/reagents/reagent_containers/syringes.dm index bc734306d19..499519c0c4a 100644 --- a/code/modules/reagents/reagent_containers/syringes.dm +++ b/code/modules/reagents/reagent_containers/syringes.dm @@ -199,6 +199,7 @@ name = "lethal injection syringe" desc = "A syringe used for lethal injections. It can hold up to 50 units." amount_per_transfer_from_this = 50 + has_variable_transfer_amount = FALSE volume = 50 /obj/item/reagent_containers/syringe/lethal/choral @@ -211,6 +212,7 @@ name = "Mulligan" desc = "A syringe used to completely change the users identity." amount_per_transfer_from_this = 1 + has_variable_transfer_amount = FALSE volume = 1 list_reagents = list(/datum/reagent/mulligan = 1) @@ -218,6 +220,7 @@ name = "Gluttony's Blessing" desc = "A syringe recovered from a dread place. It probably isn't wise to use." amount_per_transfer_from_this = 1 + has_variable_transfer_amount = FALSE volume = 1 list_reagents = list(/datum/reagent/gluttonytoxin = 1) diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm index eaf25edf81f..d3a5aafdf7e 100644 --- a/code/modules/unit_tests/_unit_tests.dm +++ b/code/modules/unit_tests/_unit_tests.dm @@ -183,6 +183,7 @@ #include "quirks.dm" #include "range_return.dm" #include "rcd.dm" +#include "reagent_container_defaults.dm" #include "reagent_id_typos.dm" #include "reagent_mob_expose.dm" #include "reagent_mod_procs.dm" diff --git a/code/modules/unit_tests/reagent_container_defaults.dm b/code/modules/unit_tests/reagent_container_defaults.dm new file mode 100644 index 00000000000..83df22fa36a --- /dev/null +++ b/code/modules/unit_tests/reagent_container_defaults.dm @@ -0,0 +1,12 @@ +/// Checks if reagent container transfer amount defaults match with actual possible values +/datum/unit_test/reagent_container_defaults + +/datum/unit_test/reagent_container_defaults/Run() + for(var/container_type in subtypesof(/obj/item/reagent_containers)) + var/obj/item/reagent_containers/container = allocate(container_type) + if(!container.has_variable_transfer_amount) + continue + var/initial_value = initial(container.amount_per_transfer_from_this) + var/index_of_initial_value = container.possible_transfer_amounts.Find(initial_value) + if(index_of_initial_value == 0) + TEST_FAIL("Reagent container [container_type]: initial value of amount_per_transfer_from_this value ([initial_value]) not found in possible_transfer_amounts list")