Fix basic mobs triggering click cooldown erroneously. (#95906)

## About The Pull Request

Basic Mobs used to always trigger cooldown on clicks. This resulted in
missed attacks causing you to have a delay before you could attack
again, this makes it really punishing to play as a basic mob.

To resolve this I did a mini-refactor on basic mob's attack chain to
allow for the return values to determine whether we go on cooldown or
not. Preventing attacks that did nothing (due to not passing checks, or
simply not having any behavior) from causing cooldown.

This fixes #95605

## Why It's Good For The Game

being able to perform melee with the same rules as /human is only fair

## Changelog

🆑 DresserOnFire
fix: Fixes a bug where player-controlled basic mobs would get a cooldown
when their attacks miss
refactor: basic mob attack chain can now decided whether an attack
resulted in a cooldown or not.
/🆑

code/modules/mob/living/basic/alien/maid.dm

@@ -19,15 +19,15 @@
 ///Handles the maid attacking other players, cancelling the attack to clean up instead.
 /mob/living/basic/alien/maid/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	target.wash(CLEAN_SCRUB)
 	if(istype(target, /obj/effect/decal/cleanable))
 		visible_message(span_notice("[src] cleans up \the [target]."))
 	else
 		visible_message(span_notice("[src] polishes \the [target]."))
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /**
  * Barmaid special type

code/modules/mob/living/basic/basic.dm

@@ -232,21 +232,22 @@
 	. += span_deadsay("Upon closer examination, [p_they()] appear[p_s()] to be [HAS_MIND_TRAIT(user, TRAIT_NAIVE) ? "asleep" : "dead"].")
 
 /mob/living/basic/proc/melee_attack(atom/target, list/modifiers, ignore_cooldown = FALSE)
-	if(!early_melee_attack(target, modifiers, ignore_cooldown))
+	var/early_melee_result = early_melee_attack(target, modifiers, ignore_cooldown)
+	if(early_melee_result) //Truthy value means we want to end the chain
+		if(!ignore_cooldown && early_melee_result == BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN)
+			changeNext_move(melee_attack_cooldown)
 		return FALSE
 	var/result = target.attack_basic_mob(src, modifiers)
 	SEND_SIGNAL(src, COMSIG_HOSTILE_POST_ATTACKINGTARGET, target, result)
-	if(!ignore_cooldown)
-		changeNext_move(melee_attack_cooldown) // Set it again because objects like to fuck with it in attack_basic_mob
+	if(result && !ignore_cooldown) //Only set cooldown if the attack achieved something, which is the case when the value is true-ey. This could definitely be done better but is probably easier once living/simple is gone and we no longer use attack_animal.
+		changeNext_move(melee_attack_cooldown)
 	return result
 
 /mob/living/basic/proc/early_melee_attack(atom/target, list/modifiers, ignore_cooldown = FALSE)
 	face_atom(target)
-	if(!ignore_cooldown)
-		changeNext_move(melee_attack_cooldown) // Set cooldown early in case it is cancelled
 	if(SEND_SIGNAL(src, COMSIG_HOSTILE_PRE_ATTACKINGTARGET, target, Adjacent(target), modifiers) & COMPONENT_HOSTILE_NO_ATTACK)
-		return FALSE //but more importantly return before attack_animal called
-	return TRUE
+		return BASIC_MOB_END_ATTACK_CHAIN //but more importantly return before attack_animal called
+	return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 /mob/living/basic/resolve_unarmed_attack(atom/attack_target, list/modifiers)
 	melee_attack(attack_target, modifiers)

code/modules/mob/living/basic/boss/boss.dm

@@ -77,10 +77,11 @@
 
 /mob/living/basic/boss/early_melee_attack(mob/living/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!. || !istype(target))
+	if(. || !istype(target))
 		return
 	if(should_devour(target))
 		devour(target)
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /// Determines if this mob is worth devouring
 /mob/living/basic/boss/proc/should_devour(mob/living/victim)

code/modules/mob/living/basic/bots/secbot/super_beepsky.dm

@@ -75,8 +75,11 @@
 
 /mob/living/basic/bot/secbot/grievous/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
+	if(.)
+		return
 	INVOKE_ASYNC(weapon, TYPE_PROC_REF(/obj/item, melee_attack_chain), src, target)
 	playsound(src, 'sound/items/weapons/blade1.ogg', 50, TRUE, -1)
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/bot/secbot/grievous/explode()
 	var/atom/drop_location = drop_location()

code/modules/mob/living/basic/farm_animals/bee/_bee.dm

@@ -111,18 +111,18 @@
 
 /mob/living/basic/bee/early_melee_attack(atom/target, list/modifiers)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(istype(target, /obj/machinery/hydroponics))
 		var/obj/machinery/hydroponics/hydro = target
 		pollinate(hydro)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(istype(target, /obj/structure/beebox))
 		var/obj/structure/beebox/hive = target
 		handle_habitation(hive)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/bee/proc/handle_habitation(obj/structure/beebox/hive)
 	if(hive == beehome) //if its our home, we enter or exit it

code/modules/mob/living/basic/icemoon/ice_whelp/ice_whelp.dm

@@ -51,22 +51,22 @@
 
 /mob/living/basic/mining/ice_whelp/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(istype(target, /obj/structure/flora/rock/icy))
 		create_sculpture(target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(!istype(target, type))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	var/mob/living/victim = target
 	if(victim.stat != DEAD)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	cannibalize_victim(victim)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /// Carve a stone into a beautiful self-portrait
 /mob/living/basic/mining/ice_whelp/proc/create_sculpture(atom/target)

code/modules/mob/living/basic/jungle/seedling/seedling.dm

@@ -85,19 +85,19 @@
 
 /mob/living/basic/seedling/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(istype(target, /obj/machinery/hydroponics))
 		treat_hydro_tray(target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(isnull(held_can))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	if(istype(target, /obj/structure/sink) || istype(target, /obj/structure/reagent_dispensers))
 		held_can.melee_attack_chain(src, target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 
 ///seedlings can water trays, remove weeds, or remove dead plants

code/modules/mob/living/basic/lavaland/gutlunchers/gutlunchers.dm

@@ -46,16 +46,16 @@
 
 /mob/living/basic/mining/gutlunch/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
+	if(.)
 		return
 	if(!istype(target, /obj/structure/ore_container/food_trough/gutlunch_trough))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	var/obj/ore_food = locate(/obj/item/stack/ore) in target
 	if(isnull(ore_food))
 		balloon_alert(src, "no food!")
 	else
 		UnarmedAttack(ore_food, TRUE, modifiers)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/mining/gutlunch/proc/after_birth(mob/living/basic/mining/gutlunch/grub/baby, mob/living/partner)
 	var/our_color = LAZYACCESS(atom_colours, FIXED_COLOUR_PRIORITY) || COLOR_GRAY

code/modules/mob/living/basic/lavaland/mook/mook.dm

@@ -97,26 +97,26 @@
 
 /mob/living/basic/mining/mook/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 	return attack_sequence(target)
 
 /mob/living/basic/mining/mook/proc/attack_sequence(atom/target)
 	if(istype(target, /obj/item/stack/ore) && isnull(held_ore))
 		var/obj/item/ore_target = target
 		ore_target.forceMove(src)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(istype(target, /obj/structure/ore_container/material_stand))
 		if(held_ore)
 			held_ore.forceMove(target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(istype(target, /obj/structure/bonfire))
 		var/obj/structure/bonfire/fire_target = target
 		if(!fire_target.burning)
 			fire_target.start_burning()
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/mining/mook/proc/change_combatant_state(state)
 	attack_state = state

code/modules/mob/living/basic/lavaland/raptor/_raptor.dm

@@ -169,16 +169,16 @@ GLOBAL_LIST_EMPTY(raptor_population)
 
 /mob/living/basic/raptor/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 	if(!istype(target, /obj/structure/ore_container/food_trough/raptor_trough))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	var/obj/ore_food = locate(/obj/item/stack/ore) in target
 	if(isnull(ore_food))
 		balloon_alert(src, "no food!")
 	else
 		UnarmedAttack(ore_food, TRUE, modifiers)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/raptor/melee_attack(mob/living/target, list/modifiers, ignore_cooldown)
 	if (!combat_mode && istype(target, /mob/living/basic/raptor))

code/modules/mob/living/basic/minebots/minebot.dm

@@ -255,13 +255,13 @@
 
 /mob/living/basic/mining_drone/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(!istype(target, /mob/living/basic/node_drone))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	repair_node_drone(target)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/mining_drone/proc/repair_node_drone(mob/living/my_target)
 	do_sparks(5, FALSE, source = my_target)

code/modules/mob/living/basic/pets/cat/cat.dm

@@ -103,21 +103,21 @@
 
 /mob/living/basic/pet/cat/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(istype(target, /obj/machinery/oven/range) && can_interact_with_stove)
 		target.attack_hand(src)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(!can_hold_item)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	if(!is_type_in_list(target, huntable_items) || held_food)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	var/atom/movable/movable_target = target
 	movable_target.forceMove(src)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/pet/cat/Exited(atom/movable/gone, direction)
 	. = ..()

code/modules/mob/living/basic/pets/orbie/orbie.dm

@@ -63,14 +63,14 @@
 
 /mob/living/basic/orbie/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 	if(src == target || happy_state || !istype(target))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	toggle_happy_state()
 	addtimer(CALLBACK(src, PROC_REF(toggle_happy_state)), 30 SECONDS)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/orbie/proc/on_lights(datum/source)
 	SIGNAL_HANDLER

code/modules/mob/living/basic/pets/parrot/_parrot.dm

@@ -282,14 +282,14 @@ GLOBAL_LIST_INIT(strippable_parrot_items, create_strippable_list(list(
 /// This is pretty much meant for players, AI will use the task-specific procs instead.
 /mob/living/basic/parrot/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(isitem(target) && steal_from_ground(target))
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(iscarbon(target) && steal_from_mob(target))
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /// Picks up an item from the ground and puts it in our claws. Returns TRUE if we picked it up, FALSE otherwise.
 /mob/living/basic/parrot/proc/steal_from_ground(obj/item/target)

code/modules/mob/living/basic/ruin_defender/mimic/mimic.dm

@@ -138,7 +138,7 @@ GLOBAL_LIST_INIT(animatable_blacklist, typecacheof(list(
 /mob/living/basic/mimic/crate/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	if(target == src)
 		toggle_open()
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 	return ..()
 
 /mob/living/basic/mimic/crate/CanAllowThrough(atom/movable/mover, border_dir)

code/modules/mob/living/basic/space_fauna/eyeball/_eyeball.dm

@@ -96,16 +96,16 @@
 
 /mob/living/basic/eyeball/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 	if(!ishuman(target))
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	var/mob/living/carbon/human_target = target
 	var/obj/item/organ/eyes/eyes = human_target.get_organ_slot(ORGAN_SLOT_EYES)
 	if(isnull(eyes) || eyes.damage < 10)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	heal_eye_damage(human_target, eyes)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/eyeball/proc/heal_eye_damage(mob/living/target, obj/item/organ/eyes/eyes)
 	if(!COOLDOWN_FINISHED(src, eye_healing))

code/modules/mob/living/basic/space_fauna/hivebot/_hivebot.dm

@@ -99,16 +99,16 @@
 
 /mob/living/basic/hivebot/mechanic/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(ismachinery(target))
 		repair_machine(target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(istype(target, /mob/living/basic/hivebot))
 		repair_hivebot(target)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /mob/living/basic/hivebot/mechanic/proc/repair_machine(obj/machinery/fixable)
 	if(fixable.get_integrity() >= fixable.max_integrity)

code/modules/mob/living/basic/space_fauna/morph.dm

@@ -149,29 +149,29 @@
 /// Handles the logic for attacking anything.
 /mob/living/basic/morph/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(HAS_TRAIT(src, TRAIT_DISGUISED) && (melee_damage_disguised <= 0))
 		balloon_alert(src, "can't attack while disguised!")
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(isliving(target)) //Eat Corpses to regen health
 		var/mob/living/living_target = target
 		if(living_target.stat != DEAD)
-			return TRUE
+			return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 		eat(eatable = living_target, delay = 3 SECONDS, update_health = -50)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(!isitem(target)) //Eat items just to be annoying
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	var/obj/item/item_target = target
 	if(item_target.anchored)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 	eat(eatable = item_target, delay = 2 SECONDS)
-	return FALSE
+	return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /// Eat stuff. Delicious. Return TRUE if we ate something, FALSE otherwise.
 /// Required: `eatable` is the thing (item or mob) that we are going to eat.

code/modules/mob/living/basic/space_fauna/regal_rat/regal_rat.dm

@@ -171,22 +171,22 @@
 /// Checks if we are able to attack this object, as well as send out the signal to see if we get any special regal rat interactions.
 /mob/living/basic/regal_rat/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(DOING_INTERACTION(src, REGALRAT_INTERACTION) || !allowed_to_attack(target))
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN
 
 	if(SEND_SIGNAL(target, COMSIG_RAT_INTERACT, src) & COMPONENT_RAT_INTERACTED)
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(isnull(mind) || combat_mode)
-		return TRUE
+		return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 	if(poison_target(target))
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
-	return TRUE
+	return BASIC_MOB_CONTINUE_ATTACK_CHAIN
 
 /// Checks if we are allowed to attack this mob. Will return TRUE if we are potentially allowed to attack, but if we end up in a case where we should NOT attack, return FALSE.
 /mob/living/basic/regal_rat/proc/allowed_to_attack(atom/the_target)

code/modules/mob/living/basic/space_fauna/supermatter_spider.dm

@@ -49,8 +49,8 @@
 /// Proc that we call on attacking something to dust 'em.
 /mob/living/basic/supermatter_spider/early_melee_attack(atom/target, list/modifiers, ignore_cooldown)
 	. = ..()
-	if(!.)
-		return FALSE
+	if(.)
+		return
 
 	if(isliving(target))
 		var/mob/living/victim = target
@@ -59,14 +59,14 @@
 		victim.dust()
 		if(single_use)
 			death()
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 	if(!isturf(target))
 		dust_feedback(target)
 		qdel(target)
 		if(single_use)
 			death()
-		return FALSE
+		return BASIC_MOB_END_ATTACK_CHAIN_COOLDOWN
 
 /// Simple proc that plays the supermatter dusting sound and sends a visible message.
 /mob/living/basic/supermatter_spider/proc/dust_feedback(atom/target)