From b438cc0039c1867e37952e25e2ae140f295db130 Mon Sep 17 00:00:00 2001 From: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Date: Thu, 16 Jun 2022 10:39:15 -0500 Subject: [PATCH] Fixes `Knockdown` signal being incorrect, making knock-off items (and others) trigger when they should again. Also cleans up the knockoff component and unit tests it. (#67720) At some point, someone did a find and replace over this file, and completely screwed up the signal for Knockdown(). This caused components that relied on it, like the Knockoff component, to work way less often. This PR fixes that. It also goes through and cleans up the Knockoff component. More consistent style guide stuff, minor improvements, better documentation. It also unit tests it. --- code/datums/components/knockoff.dm | 108 +++++++++++------- code/game/objects/items/cigs_lighters.dm | 2 +- code/modules/clothing/glasses/_glasses.dm | 2 +- code/modules/mob/living/status_procs.dm | 2 +- code/modules/unit_tests/_unit_tests.dm | 1 + code/modules/unit_tests/knockoff_component.dm | 87 ++++++++++++++ 6 files changed, 159 insertions(+), 43 deletions(-) create mode 100644 code/modules/unit_tests/knockoff_component.dm diff --git a/code/datums/components/knockoff.dm b/code/datums/components/knockoff.dm index 08f0a1f578f..5efe33af69b 100644 --- a/code/datums/components/knockoff.dm +++ b/code/datums/components/knockoff.dm @@ -1,71 +1,99 @@ -///Items with these will have a chance to get knocked off when disarming or being knocked down +/// Items with this component will have a chance to get knocked off +/// (unequipped and sent to the ground) when the wearer is disarmed or knocked down. /datum/component/knockoff - ///Chance to knockoff + /// Chance to knockoff when a knockoff action occurs. var/knockoff_chance = 100 - ///Aiming for these zones will cause the knockoff, null means all zones allowed + /// Used in being disarmed. + /// If set, we will only roll the knockoff chance if the disarmer is targeting one of these zones. + /// If unset, any disarm act will cause the knock-off chance to be rolled, no matter the zone targeted. var/list/target_zones - ///Can be only knocked off from these slots, null means all slots allowed - var/list/slots_knockoffable + /// Bitflag used in equip to determine what slots we need to be in to be knocked off. + /// If set, we must be equipped in one of the slots to have a chance of our item being knocked off. + /// If unset / NONE, a disarm or knockdown will have a chance of our item being knocked off regardless of slot, INCLUDING hand slots. + var/slots_knockoffable = NONE -/datum/component/knockoff/Initialize(knockoff_chance,zone_override,slots_knockoffable) +/datum/component/knockoff/Initialize(knockoff_chance = 100, target_zones, slots_knockoffable = NONE) if(!isitem(parent)) return COMPONENT_INCOMPATIBLE - RegisterSignal(parent, COMSIG_ITEM_EQUIPPED,.proc/OnEquipped) - RegisterSignal(parent, COMSIG_ITEM_DROPPED,.proc/OnDropped) src.knockoff_chance = knockoff_chance + src.target_zones = target_zones + src.slots_knockoffable = slots_knockoffable - if(zone_override) - target_zones = zone_override +/datum/component/knockoff/RegisterWithParent() + RegisterSignal(parent, COMSIG_ITEM_EQUIPPED, .proc/on_equipped) + RegisterSignal(parent, COMSIG_ITEM_DROPPED, .proc/on_dropped) - if(slots_knockoffable) - src.slots_knockoffable = slots_knockoffable +/datum/component/knockoff/UnregisterFromParent() + UnregisterSignal(parent, list(COMSIG_ITEM_EQUIPPED, COMSIG_ITEM_DROPPED)) -///Tries to knockoff the item when disarmed -/datum/component/knockoff/proc/Knockoff(mob/living/carbon/human/wearer,mob/living/attacker,zone) + var/obj/item/item_parent = parent + if(ismob(item_parent.loc)) + UnregisterSignal(item_parent.loc, list(COMSIG_HUMAN_DISARM_HIT, COMSIG_LIVING_STATUS_KNOCKDOWN)) + +/// Signal proc for [COMSIG_HUMAN_DISARM_HIT] on the mob who's equipped our parent +/// Rolls a chance for knockoff whenever we're disarmed +/datum/component/knockoff/proc/on_equipped_mob_disarm(mob/living/carbon/human/source, mob/living/attacker, zone) SIGNAL_HANDLER - var/obj/item/item = parent - if(!istype(wearer)) + if(!istype(source)) return + if(target_zones && !(zone in target_zones)) return if(!prob(knockoff_chance)) return - if(!wearer.dropItemToGround(item)) - return - wearer.visible_message(span_warning("[attacker] knocks off [wearer]'s [item.name]!"),span_userdanger("[attacker] knocks off your [item.name]!")) -///Tries to knockoff the item when user is knocked down -/datum/component/knockoff/proc/Knockoff_knockdown(mob/living/carbon/human/wearer,amount) + var/obj/item/item_parent = parent + if(!source.dropItemToGround(item_parent)) + return + + source.visible_message( + span_warning("[attacker] knocks off [source]'s [item_parent.name]!"), + span_userdanger("[attacker] knocks off your [item_parent.name]!"), + ) + +/// Signal proc for [COMSIG_LIVING_STATUS_KNOCKDOWN] on the mob who's equipped our parent +/// Rolls a chance for knockoff whenever we're knocked down +/datum/component/knockoff/proc/on_equipped_mob_knockdown(mob/living/carbon/human/source, amount) SIGNAL_HANDLER - if(amount <= 0) + if(!istype(source)) return - var/obj/item/item = parent - if(!istype(wearer)) + // Healing knockdown or setting knockdown to zero or something? Don't knock off. + if(amount <= 0) return if(!prob(knockoff_chance)) return - if(!wearer.dropItemToGround(item)) - return - wearer.visible_message(span_warning("[wearer]'s [item.name] get[item.p_s()] knocked off!"),span_userdanger("Your [item.name] [item.p_were()] knocked off!")) - -/datum/component/knockoff/proc/OnEquipped(datum/source, mob/living/carbon/human/H,slot) - SIGNAL_HANDLER - if(!istype(H)) + var/obj/item/item_parent = parent + if(!source.dropItemToGround(item_parent)) return - if(slots_knockoffable && !(slot in slots_knockoffable)) - UnregisterSignal(H, COMSIG_HUMAN_DISARM_HIT) - UnregisterSignal(H, COMSIG_LIVING_STATUS_KNOCKDOWN) - return - RegisterSignal(H, COMSIG_HUMAN_DISARM_HIT, .proc/Knockoff, TRUE) - RegisterSignal(H, COMSIG_LIVING_STATUS_KNOCKDOWN, .proc/Knockoff_knockdown, TRUE) -/datum/component/knockoff/proc/OnDropped(datum/source, mob/living/M) + source.visible_message( + span_warning("[source]'s [item_parent.name] get[item_parent.p_s()] knocked off!"), + span_userdanger("Your [item_parent.name] [item_parent.p_were()] knocked off!"), + ) + +/// Signal proc for [COMSIG_ITEM_EQUIPPED] +/// Registers our signals which can cause a knockdown whenever we're equipped correctly +/datum/component/knockoff/proc/on_equipped(datum/source, mob/living/carbon/human/equipper, slot) SIGNAL_HANDLER - UnregisterSignal(M, COMSIG_HUMAN_DISARM_HIT) - UnregisterSignal(M, COMSIG_LIVING_STATUS_KNOCKDOWN) + if(!istype(equipper)) + return + + if(slots_knockoffable && !(slot & slots_knockoffable)) + UnregisterSignal(equipper, list(COMSIG_HUMAN_DISARM_HIT, COMSIG_LIVING_STATUS_KNOCKDOWN)) + return + + RegisterSignal(equipper, COMSIG_HUMAN_DISARM_HIT, .proc/on_equipped_mob_disarm, TRUE) + RegisterSignal(equipper, COMSIG_LIVING_STATUS_KNOCKDOWN, .proc/on_equipped_mob_knockdown, TRUE) + +/// Signal proc for [COMSIG_ITEM_DROPPED] +/// Unregisters our signals which can cause a knockdown when we're unequipped (dropped) +/datum/component/knockoff/proc/on_dropped(datum/source, mob/living/dropper) + SIGNAL_HANDLER + + UnregisterSignal(dropper, list(COMSIG_HUMAN_DISARM_HIT, COMSIG_LIVING_STATUS_KNOCKDOWN)) diff --git a/code/game/objects/items/cigs_lighters.dm b/code/game/objects/items/cigs_lighters.dm index b405b825d6e..80946f6e048 100644 --- a/code/game/objects/items/cigs_lighters.dm +++ b/code/game/objects/items/cigs_lighters.dm @@ -171,7 +171,7 @@ CIGARETTE PACKETS ARE IN FANCY.DM reagents.add_reagent_list(list_reagents) if(starts_lit) light() - AddComponent(/datum/component/knockoff, 90, list(BODY_ZONE_PRECISE_MOUTH), list(ITEM_SLOT_MASK)) //90% to knock off when wearing a mask + AddComponent(/datum/component/knockoff, 90, list(BODY_ZONE_PRECISE_MOUTH), slot_flags) //90% to knock off when wearing a mask AddElement(/datum/element/update_icon_updates_onmob) icon_state = icon_off inhand_icon_state = inhand_icon_off diff --git a/code/modules/clothing/glasses/_glasses.dm b/code/modules/clothing/glasses/_glasses.dm index f509c8625ed..14c3e18e83f 100644 --- a/code/modules/clothing/glasses/_glasses.dm +++ b/code/modules/clothing/glasses/_glasses.dm @@ -227,7 +227,7 @@ /obj/item/clothing/glasses/regular/Initialize(mapload) . = ..() - AddComponent(/datum/component/knockoff,25,list(BODY_ZONE_PRECISE_EYES),list(ITEM_SLOT_EYES)) + AddComponent(/datum/component/knockoff, 25, list(BODY_ZONE_PRECISE_EYES), slot_flags) var/static/list/loc_connections = list( COMSIG_ATOM_ENTERED = .proc/on_entered, ) diff --git a/code/modules/mob/living/status_procs.dm b/code/modules/mob/living/status_procs.dm index e0f8ccc40ba..bc186c61d76 100644 --- a/code/modules/mob/living/status_procs.dm +++ b/code/modules/mob/living/status_procs.dm @@ -71,7 +71,7 @@ return 0 /mob/living/proc/Knockdown(amount, ignore_canstun = FALSE) //Can't go below remaining duration - if(SEND_SIGNAL(src, /datum/status_effect/incapacitating/knockdown, amount, ignore_canstun) & COMPONENT_NO_STUN) + if(SEND_SIGNAL(src, COMSIG_LIVING_STATUS_KNOCKDOWN, amount, ignore_canstun) & COMPONENT_NO_STUN) return if(IS_STUN_IMMUNE(src, ignore_canstun)) return diff --git a/code/modules/unit_tests/_unit_tests.dm b/code/modules/unit_tests/_unit_tests.dm index 66f11c64049..6bbbe23804d 100644 --- a/code/modules/unit_tests/_unit_tests.dm +++ b/code/modules/unit_tests/_unit_tests.dm @@ -101,6 +101,7 @@ #include "hydroponics_self_mutations.dm" #include "hydroponics_validate_genes.dm" #include "keybinding_init.dm" +#include "knockoff_component.dm" #include "load_map_security.dm" #include "machine_disassembly.dm" #include "mapping.dm" diff --git a/code/modules/unit_tests/knockoff_component.dm b/code/modules/unit_tests/knockoff_component.dm new file mode 100644 index 00000000000..a781fb9eb0d --- /dev/null +++ b/code/modules/unit_tests/knockoff_component.dm @@ -0,0 +1,87 @@ +/// Test that the knockoff component will properly cause something +/// with it applied to be knocked off when it should be. +/datum/unit_test/knockoff_component + +/datum/unit_test/knockoff_component/Run() + var/mob/living/carbon/human/wears_the_glasses = allocate(/mob/living/carbon/human) + var/mob/living/carbon/human/shoves_the_guy = allocate(/mob/living/carbon/human) + + // No pre-existing items have a 100% chance of being knocked off, + // so we'll just apply it to a relatively generic item (glasses) + var/obj/item/clothing/glasses/sunglasses/glasses = allocate(/obj/item/clothing/glasses/sunglasses) + glasses.AddComponent(/datum/component/knockoff, \ + knockoff_chance = 100, \ + target_zones = list(BODY_ZONE_PRECISE_EYES), \ + slots_knockoffable = glasses.slot_flags) + + // Save this for later, since we wanna reset our dummy positions even after they're shoved about. + var/turf/right_of_shover = locate(run_loc_floor_bottom_left.x + 1, run_loc_floor_bottom_left.y, run_loc_floor_bottom_left.z) + + // Position shover (bottom left) and the shovee (1 tile right of bottom left, no wall behind them) + shoves_the_guy.forceMove(run_loc_floor_bottom_left) + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + TEST_ASSERT(wears_the_glasses.glasses == glasses, "Dummy failed to equip the glasses.") + + // Test disarm, targeting chest + // A disarm targeting chest should not knockdown or lose glasses + shoves_the_guy.zone_selected = BODY_ZONE_CHEST + shoves_the_guy.disarm(wears_the_glasses) + TEST_ASSERT(!wears_the_glasses.IsKnockdown(), "Dummy was knocked down when being disarmed shouldn't have been.") + TEST_ASSERT(wears_the_glasses.glasses == glasses, "Dummy lost their glasses even thought they were disarmed targeting the wrong slot.") + + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + // Test disarm, targeting eyes + // A disarm targeting eyes should not knockdown but should lose glasses + shoves_the_guy.zone_selected = BODY_ZONE_PRECISE_EYES + shoves_the_guy.disarm(wears_the_glasses) + TEST_ASSERT(!wears_the_glasses.IsKnockdown(), "Dummy was knocked down when being disarmed shouldn't have been.") + TEST_ASSERT(wears_the_glasses.glasses != glasses, "Dummy kept their glasses, even though they were shoved targeting the correct zone.") + + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + // Test Knockdown() + // Any amount of positive Kockdown should lose glasses + wears_the_glasses.Knockdown(1 SECONDS) + TEST_ASSERT(wears_the_glasses.IsKnockdown(), "Dummy wasn't knocked down after Knockdown() was called.") + TEST_ASSERT(wears_the_glasses.glasses != glasses, "Dummy kept their glasses, even though they knocked down by Knockdown().") + + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + // Test AdjustKnockdown() + // Any amount of positive Kockdown should lose glasses + wears_the_glasses.AdjustKnockdown(1 SECONDS) + TEST_ASSERT(wears_the_glasses.IsKnockdown(), "Dummy wasn't knocked down after AdjustKnockdown() was called.") + TEST_ASSERT(wears_the_glasses.glasses != glasses, "Dummy kept their glasses, even though they knocked down by AdjustKnockdown().") + + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + // Test SetKnockdown() + // Any amount of positive Kockdown should lose glasses + wears_the_glasses.SetKnockdown(1 SECONDS) + TEST_ASSERT(wears_the_glasses.IsKnockdown(), "Dummy wasn't knocked down after SetKnockdown() was called.") + TEST_ASSERT(wears_the_glasses.glasses != glasses, "Dummy kept their glasses, even though they knocked down by SetKnockdown().") + + set_glasses_wearer(wears_the_glasses, right_of_shover, glasses) + + // Test a negative value applied of Knockdown (AdjustKnockdown, SetKnockdown, and Knockdown should all act the same here) + // Any amount of negative Kockdown should not cause the glasses to be lost + wears_the_glasses.AdjustKnockdown(-1 SECONDS) + TEST_ASSERT(!wears_the_glasses.IsKnockdown(), "Dummy was knocked down after AdjustKnockdown() was called with a negative value.") + TEST_ASSERT(wears_the_glasses.glasses == glasses, "Dummy lost their glasses, even though AdjustKnockdown() was called with a negative value.") + + // Bonus check: A wallshove should definitely cause them to be lost + wears_the_glasses.forceMove(shoves_the_guy.loc) + shoves_the_guy.forceMove(right_of_shover) + + shoves_the_guy.zone_selected = BODY_ZONE_CHEST + shoves_the_guy.disarm(wears_the_glasses) + TEST_ASSERT(wears_the_glasses.glasses != glasses, "Dummy kept their glasses, even though were disarm shoved into a wall.") + +/// Helper to reset the glasses dummy back to it's original position, clear knockdown, and return glasses (if gone) +/datum/unit_test/knockoff_component/proc/set_glasses_wearer(mob/living/carbon/human/wearer, turf/reset_to, obj/item/clothing/glasses/reset_worn) + wearer.forceMove(reset_to) + wearer.SetKnockdown(0 SECONDS) + if(!wearer.glasses) + wearer.equip_to_slot_if_possible(reset_worn, ITEM_SLOT_EYES)