From 57884727caa5f0677ed06b1414528e36668d7ec8 Mon Sep 17 00:00:00 2001 From: MrMelbert <51863163+MrMelbert@users.noreply.github.com> Date: Thu, 14 Aug 2025 21:24:18 -0500 Subject: [PATCH] Attackedby differentiates failed attacks from 0 damage attacks (#92564) ## About The Pull Request Fixes #92558 Currently `attacked_by` does not differentiate an attack that did 0 damage with an attack that failed (due to blocking or whatnot) See also: This hack I left in https://github.com/tgstation/tgstation/blob/ce958c77c006a5fe279fb46fed513206e341cfce/code/_onclick/item_attack.dm#L346-L347 This causes problems because successful attacks can deal 0 damage. See linked issue. This PR addresses the issue by having `attacked_by` return `-1` (`ATTACK_FAILED`) for attacks which entirely do not connect. -1 was used so consumers can easily check if an attack did 0 damage OR failed (via checking `<= 0`) This isn't the preferred fix - I'd prefer if all block checking and zone targeting was moved to `/item/proc/attack`, but that requires attack itself be reigned in a bit (cuz it's still a bit of a mess). ## Changelog :cl: Melbert fix: Item on-attack effects will trigger as expected when hitting a limb at damage cap /:cl: --- code/__DEFINES/combat.dm | 4 ++++ code/_onclick/item_attack.dm | 15 +++++++++------ code/game/machinery/firealarm.dm | 2 +- code/game/objects/structures/mirror.dm | 2 +- .../machinery/portable/portable_atmospherics.dm | 2 +- code/modules/power/lighting/light.dm | 2 +- 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/code/__DEFINES/combat.dm b/code/__DEFINES/combat.dm index b202ffbd2ee..982f83e9d06 100644 --- a/code/__DEFINES/combat.dm +++ b/code/__DEFINES/combat.dm @@ -433,5 +433,9 @@ BUBBERSTATION CHANGE END */ #define CALCULATE_FORCE(some_item, atk_mods) \ ((((FORCE_OVERRIDE in atk_mods) ? atk_mods[FORCE_OVERRIDE] : some_item.force) + (atk_mods?[FORCE_MODIFIER] || 0)) * ((FORCE_MULTIPLIER in atk_mods) ? atk_mods[FORCE_MULTIPLIER] : 1)) +/// Return from attacked_by to indicate the attack did not connect +/// A negative number is used here to people can easily check "attacks that failed or did 0 damage" with <= 0 +#define ATTACK_FAILED -1 + ///Do we block carbon-level flash_act() from performing its default stamina damage/knockdown? #define FLASH_COMPLETED "flash_completed" diff --git a/code/_onclick/item_attack.dm b/code/_onclick/item_attack.dm index 2b95b96df38..9f78a492ab9 100644 --- a/code/_onclick/item_attack.dm +++ b/code/_onclick/item_attack.dm @@ -246,7 +246,7 @@ if(get(src, /mob/living) == user) // telekinesis. user.do_attack_animation(target_mob) - if(!target_mob.attacked_by(src, user, modifiers, attack_modifiers)) + if(target_mob.attacked_by(src, user, modifiers, attack_modifiers) == ATTACK_FAILED) return TRUE SEND_SIGNAL(src, COMSIG_ITEM_AFTERATTACK, target_mob, user, modifiers, attack_modifiers) @@ -281,7 +281,8 @@ user.changeNext_move(attack_speed) if(get(src, /mob/living) == user) // telekinesis. user.do_attack_animation(attacked_atom) - attacked_atom.attacked_by(src, user, modifiers, attack_modifiers) + if(attacked_atom.attacked_by(src, user, modifiers, attack_modifiers) == ATTACK_FAILED) + return TRUE SEND_SIGNAL(src, COMSIG_ITEM_AFTERATTACK, attacked_atom, user, modifiers, attack_modifiers) SEND_SIGNAL(attacked_atom, COMSIG_ATOM_AFTER_ATTACKEDBY, src, user, modifiers, attack_modifiers) afterattack(attacked_atom, user, modifiers, attack_modifiers) @@ -290,7 +291,8 @@ /// Called from [/obj/item/proc/attack_atom] and [/obj/item/proc/attack] if the attack succeeds /atom/proc/attacked_by(obj/item/attacking_item, mob/living/user, list/modifiers, list/attack_modifiers) if(!uses_integrity) - CRASH("attacked_by() was called on an object that doesn't use integrity!") + stack_trace("attacked_by() was called on an object that doesn't use integrity!") + return ATTACK_FAILED var/final_force = CALCULATE_FORCE(attacking_item, attack_modifiers) if(final_force <= 0) @@ -304,7 +306,8 @@ return damage /area/attacked_by(obj/item/attacking_item, mob/living/user, list/modifiers, list/attack_modifiers) - CRASH("areas are NOT supposed to have attacked_by() called on them!") + stack_trace("areas are NOT supposed to have attacked_by() called on them!") + return ATTACK_FAILED /mob/living/attacked_by(obj/item/attacking_item, mob/living/user, list/modifiers, list/attack_modifiers) @@ -339,12 +342,12 @@ if(user != src) // This doesn't factor in armor, or most damage modifiers (physiology). Your mileage may vary if(check_block(attacking_item, final_force, "\the [attacking_item]", MELEE_ATTACK, attacking_item.armour_penetration, attacking_item.damtype)) - return 0 + return ATTACK_FAILED SEND_SIGNAL(attacking_item, COMSIG_ITEM_ATTACK_ZONE, src, user, targeting) if(final_force <= 0) - return 1 // Pretend like we did 1 damage so afterattack still runs + return 0 if(ishuman(src) || client) // istype(src) is kinda bad, but it's to avoid spamming the blackbox SSblackbox.record_feedback("nested tally", "item_used_for_combat", 1, list("[attacking_item.force]", "[attacking_item.type]")) diff --git a/code/game/machinery/firealarm.dm b/code/game/machinery/firealarm.dm index 61d880a1597..1b5e012d2fb 100644 --- a/code/game/machinery/firealarm.dm +++ b/code/game/machinery/firealarm.dm @@ -517,7 +517,7 @@ // Taking melee damage always triggers the alarm if panel is open /obj/machinery/firealarm/attacked_by(obj/item/attacking_item, mob/living/user, list/modifiers, list/attack_modifiers) . = ..() - if(!. || !panel_open || buildstage != FIRE_ALARM_BUILD_SECURED) + if(. <= 0 || !panel_open || buildstage != FIRE_ALARM_BUILD_SECURED) return alarm() diff --git a/code/game/objects/structures/mirror.dm b/code/game/objects/structures/mirror.dm index 2085b35878d..e6ccd1de1b9 100644 --- a/code/game/objects/structures/mirror.dm +++ b/code/game/objects/structures/mirror.dm @@ -277,7 +277,7 @@ MAPPING_DIRECTIONAL_HELPERS(/obj/structure/mirror/broken, 28) /obj/structure/mirror/attacked_by(obj/item/I, mob/living/user, list/modifiers, list/attack_modifiers) . = ..() - if(broken || !.) // breaking a mirror truly gets you bad luck! + if(broken || . <= 0) // breaking a mirror truly gets you bad luck! return to_chat(user, span_warning("A chill runs down your spine as [src] shatters...")) user.AddComponent(/datum/component/omen, incidents_left = 7) diff --git a/code/modules/atmospherics/machinery/portable/portable_atmospherics.dm b/code/modules/atmospherics/machinery/portable/portable_atmospherics.dm index 8e5fccbde95..7926b5b4173 100644 --- a/code/modules/atmospherics/machinery/portable/portable_atmospherics.dm +++ b/code/modules/atmospherics/machinery/portable/portable_atmospherics.dm @@ -315,7 +315,7 @@ /obj/machinery/portable_atmospherics/attacked_by(obj/item/item, mob/user) . = ..() - if(!.) + if(. <= 0) return investigate_log("was smacked with \a [item] by [key_name(user)].", INVESTIGATE_ATMOS) add_hiddenprint(user) diff --git a/code/modules/power/lighting/light.dm b/code/modules/power/lighting/light.dm index d734c346076..b2a28815890 100644 --- a/code/modules/power/lighting/light.dm +++ b/code/modules/power/lighting/light.dm @@ -471,7 +471,7 @@ /obj/machinery/light/attacked_by(obj/item/attacking_object, mob/living/user, list/modifiers, list/attack_modifiers) . = ..() - if(!.) + if(. <= 0) return if(status != LIGHT_BROKEN && status != LIGHT_EMPTY) return