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

🆑 Melbert
fix: Item on-attack effects will trigger as expected when hitting a limb
at damage cap
/🆑

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"

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]"))

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()
 

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)

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)

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