Merge pull request #203 from jamilnielsen/replacing_weakref_TG_port_b…

…y_MrMelbert

Replacing weakrefs in AI blackboard with deleting signals (TG PORT)

code/datums/ai/_ai_controller.dm

@@ -7,6 +7,16 @@ multiple modular subtrees with behaviors
 /datum/ai_controller
 	///The atom this controller is controlling
 	var/atom/pawn
+	/**
+	 * This is a list of variables the AI uses and can be mutated by actions.
+	 *
+	 * When an action is performed you pass this list and any relevant keys for the variables it can mutate.
+	 *
+	 * DO NOT set values in the blackboard directly, and especially not if you're adding a datum reference to this!
+	 * Use the setters, this is important for reference handing.
+	 */
+	var/list/blackboard = list()
+
 	///Bitfield of traits for this AI to handle extra behavior
 	var/ai_traits = NONE
 	///Current actions planned to be performed by the AI in the upcoming plan
@@ -21,8 +31,6 @@ multiple modular subtrees with behaviors
 	var/atom/current_movement_target
 	///Identifier for what last touched our movement target, so it can be cleared conditionally
 	var/movement_target_source
-	///This is a list of variables the AI uses and can be mutated by actions. When an action is performed you pass this list and any relevant keys for the variables it can mutate.
-	var/list/blackboard = list()
 	///Stored arguments for behaviors given during their initial creation
 	var/list/behavior_args = list()
 	///Tracks recent pathing attempts, if we fail too many in a row we fail our current plans.
@@ -336,3 +344,248 @@ multiple modular subtrees with behaviors
 		if(iter_behavior.required_distance < minimum_distance)
 			minimum_distance = iter_behavior.required_distance
 	return minimum_distance
+
+/**
+ * Used to manage references to datum by AI controllers
+ *
+ * * tracked_datum - something being added to an ai blackboard
+ * * key - the associated key
+ */
+#define TRACK_AI_DATUM_TARGET(tracked_datum, key) do { \
+	if(isweakref(tracked_datum)) { \
+		var/datum/weakref/_bad_weakref = tracked_datum; \
+		stack_trace("Weakref (Actual datum: [_bad_weakref.resolve()]) found in ai datum blackboard! \
+			This is an outdated method of ai reference handling, please remove it."); \
+	}; \
+	else if(isdatum(tracked_datum)) { \
+		var/datum/_tracked_datum = tracked_datum; \
+		if(!HAS_TRAIT_FROM(_tracked_datum, TRAIT_AI_TRACKING, "[REF(src)]_[key]")) { \
+			RegisterSignal(_tracked_datum, COMSIG_PARENT_QDELETING, PROC_REF(sig_remove_from_blackboard), override = TRUE); \
+			ADD_TRAIT(_tracked_datum, TRAIT_AI_TRACKING, "[REF(src)]_[key]"); \
+		}; \
+	}; \
+} while(FALSE)
+
+/**
+ * Used to clear previously set reference handing by AI controllers
+ *
+ * * tracked_datum - something being removed from an ai blackboard
+ * * key - the associated key
+ */
+#define CLEAR_AI_DATUM_TARGET(tracked_datum, key) do { \
+	if(isdatum(tracked_datum)) { \
+		var/datum/_tracked_datum = tracked_datum; \
+		REMOVE_TRAIT(_tracked_datum, TRAIT_AI_TRACKING, "[REF(src)]_[key]"); \
+		if(!HAS_TRAIT(_tracked_datum, TRAIT_AI_TRACKING)) { \
+			UnregisterSignal(_tracked_datum, COMSIG_PARENT_QDELETING); \
+		}; \
+	}; \
+} while(FALSE)
+
+/// Used for above to track all the keys that have registered a signal
+#define TRAIT_AI_TRACKING "tracked_by_ai"
+
+/**
+ * Sets the key to the passed "thing".
+ *
+ * * key - A blackboard key
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/set_blackboard_key(key, thing)
+	// Assume it is an error when trying to set a value overtop a list
+	if(islist(blackboard[key]))
+		CRASH("set_blackboard_key attempting to set a blackboard value to key [key] when it's a list!")
+
+	// Clear existing values
+	if(!isnull(blackboard[key]))
+		clear_blackboard_key(key)
+
+	TRACK_AI_DATUM_TARGET(thing, key)
+	blackboard[key] = thing
+
+/**
+ * Sets the key at index thing to the passed value
+ *
+ * Assumes the key value is already a list, if not throws an error.
+ *
+ * * key - A blackboard key, with its value set to a list
+ * * thing - a value which becomes the inner list value's key
+ * * value - what to set the inner list's value to
+ */
+/datum/ai_controller/proc/set_blackboard_key_assoc(key, thing, value)
+	if(!islist(blackboard[key]))
+		CRASH("set_blackboard_key_assoc called on non-list key [key]!")
+	TRACK_AI_DATUM_TARGET(thing, key)
+	TRACK_AI_DATUM_TARGET(value, key)
+	blackboard[key][thing] = value
+
+/**
+ * Similar to [proc/set_blackboard_key_assoc] but operates under the assumption the key is a lazylist (so it will create a list)
+ * More dangerous / easier to override values, only use when you want to use a lazylist
+ *
+ * * key - A blackboard key, with its value set to a list
+ * * thing - a value which becomes the inner list value's key
+ * * value - what to set the inner list's value to
+ */
+/datum/ai_controller/proc/set_blackboard_key_assoc_lazylist(key, thing, value)
+	LAZYINITLIST(blackboard[key])
+	TRACK_AI_DATUM_TARGET(thing, key)
+	TRACK_AI_DATUM_TARGET(value, key)
+	blackboard[key][thing] = value
+
+/**
+ * Adds the passed "thing" to the associated key
+ *
+ * Works with lists or numbers, but not lazylists.
+ *
+ * * key - A blackboard key
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/add_blackboard_key(key, thing)
+	TRACK_AI_DATUM_TARGET(thing, key)
+	blackboard[key] += thing
+
+/**
+ * Similar to [proc/add_blackboard_key], but performs an insertion rather than an add
+ * Throws an error if the key is not a list already, intended only for use with lists
+ *
+ * * key - A blackboard key, with its value set to a list
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/insert_blackboard_key(key, thing)
+	if(!islist(blackboard[key]))
+		CRASH("insert_blackboard_key called on non-list key [key]!")
+	TRACK_AI_DATUM_TARGET(thing, key)
+	blackboard[key] |= thing
+
+/**
+ * Adds the passed "thing" to the associated key, assuming key is intended to be a lazylist (so it will create a list)
+ * More dangerous / easier to override values, only use when you want to use a lazylist
+ *
+ * * key - A blackboard key
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/add_blackboard_key_lazylist(key, thing)
+	LAZYINITLIST(blackboard[key])
+	TRACK_AI_DATUM_TARGET(thing, key)
+	blackboard[key] += thing
+
+/**
+ * Similar to [proc/insert_blackboard_key_lazylist], but performs an insertion / or rather than an add
+ *
+ * * key - A blackboard key
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/insert_blackboard_key_lazylist(key, thing)
+	LAZYINITLIST(blackboard[key])
+	TRACK_AI_DATUM_TARGET(thing, key)
+	blackboard[key] |= thing
+
+/**
+ * Adds the value to the inner list at key with the inner key set to "thing"
+ * Throws an error if the key is not a list already, intended only for use with lists
+ *
+ * * key - A blackboard key, with its value set to a list
+ * * thing - a value which becomes the inner list value's key
+ * * value - what to set the inner list's value to
+ */
+/datum/ai_controller/proc/add_blackboard_key_assoc(key, thing, value)
+	if(!islist(blackboard[key]))
+		CRASH("add_blackboard_key_assoc called on non-list key [key]!")
+	TRACK_AI_DATUM_TARGET(thing, key)
+	TRACK_AI_DATUM_TARGET(value, key)
+	blackboard[key][thing] += value
+
+
+/**
+ * Similar to [proc/add_blackboard_key_assoc], assuming key is intended to be a lazylist (so it will create a list)
+ * More dangerous / easier to override values, only use when you want to use a lazylist
+ *
+ * * key - A blackboard key, with its value set to a list
+ * * thing - a value which becomes the inner list value's key
+ * * value - what to set the inner list's value to
+ */
+/datum/ai_controller/proc/add_blackboard_key_assoc_lazylist(key, thing, value)
+	LAZYINITLIST(blackboard[key])
+	TRACK_AI_DATUM_TARGET(thing, key)
+	TRACK_AI_DATUM_TARGET(value, key)
+	blackboard[key][thing] += value
+
+/**
+ * Clears the passed key, resetting it to null
+ *
+ * Not intended for use with list keys - use [proc/remove_thing_from_blackboard_key] if you are removing a value from a list at a key
+ *
+ * * key - A blackboard key
+ */
+/datum/ai_controller/proc/clear_blackboard_key(key)
+	CLEAR_AI_DATUM_TARGET(blackboard[key], key)
+	blackboard[key] = null
+
+/**
+ * Remove the passed thing from the associated blackboard key
+ *
+ * Intended for use with lists, if you're just clearing a reference from a key use [proc/clear_blackboard_key]
+ *
+ * * key - A blackboard key
+ * * thing - a value to set the blackboard key to.
+ */
+/datum/ai_controller/proc/remove_thing_from_blackboard_key(key, thing)
+	var/associated_value = blackboard[key]
+	if(thing == associated_value)
+		stack_trace("remove_thing_from_blackboard_key was called un-necessarily in a situation where clear_blackboard_key would suffice. ")
+		clear_blackboard_key(key)
+		return
+
+	if(!islist(associated_value))
+		CRASH("remove_thing_from_blackboard_key called with an invalid \"thing\" argument ([thing]). \
+			(The associated value of the passed key is not a list and is also not the passed thing, meaning it is clearing an unintended value.)")
+
+	for(var/inner_key in associated_value)
+		if(inner_key == thing)
+			// flat list
+			CLEAR_AI_DATUM_TARGET(thing, key)
+			associated_value -= thing
+			return
+		else if(associated_value[inner_key] == thing)
+			// assoc list
+			CLEAR_AI_DATUM_TARGET(thing, key)
+			associated_value -= inner_key
+			return
+
+	CRASH("remove_thing_from_blackboard_key called with an invalid \"thing\" argument ([thing]). \
+		(The passed value is not tracked in the passed list.)")
+
+/// Signal proc to go through every key and remove the datum from all keys it finds
+/datum/ai_controller/proc/sig_remove_from_blackboard(datum/source)
+	SIGNAL_HANDLER
+
+	var/list/list/remove_queue = list(blackboard)
+	var/index = 1
+	while(index <= length(remove_queue))
+		var/list/next_to_clear = remove_queue[index]
+		for(var/inner_value in next_to_clear)
+			var/associated_value = next_to_clear[inner_value]
+			// We are a lists of lists, add the next value to the queue so we can handle references in there
+			// (But we only need to bother checking the list if it's not empty.)
+			if(islist(inner_value) && length(inner_value))
+				UNTYPED_LIST_ADD(remove_queue, inner_value)
+
+			// We found the value that's been deleted. Clear it out from this list
+			else if(inner_value == source)
+				next_to_clear -= inner_value
+
+			// We are an assoc lists of lists, the list at the next value so we can handle references in there
+			// (But again, we only need to bother checking the list if it's not empty.)
+			if(islist(associated_value) && length(associated_value))
+				UNTYPED_LIST_ADD(remove_queue, associated_value)
+
+			// We found the value that's been deleted, it was an assoc value. Clear it out entirely
+			else if(associated_value == source)
+				next_to_clear -= inner_value
+
+		index += 1
+
+#undef TRACK_AI_DATUM_TARGET
+#undef CLEAR_AI_DATUM_TARGET
+#undef TRAIT_AI_TRACKING

code/datums/ai/_item_behaviors.dm

@@ -24,22 +24,20 @@
 
 /datum/ai_behavior/item_move_close_and_attack/setup(datum/ai_controller/controller, target_key, throw_count_key)
 	. = ..()
-	var/datum/weakref/target_ref = controller.blackboard[target_key]
-	var/atom/target = target_ref?.resolve()
+	var/atom/target = controller.blackboard[target_key]
 	if (isnull(target))
 		return FALSE
 	set_movement_target(controller, target)
 
 /datum/ai_behavior/item_move_close_and_attack/perform(seconds_per_tick, datum/ai_controller/controller, target_key, throw_count_key)
 	. = ..()
 	var/obj/item/item_pawn = controller.pawn
-	var/datum/weakref/target_ref = controller.blackboard[target_key]
-	var/atom/throw_target = target_ref?.resolve()
+	var/atom/throw_target = controller.blackboard[target_key]
 
 	item_pawn.visible_message(span_warning("[item_pawn] hurls towards [throw_target]!"))
 	item_pawn.throw_at(throw_target, rand(4,5), 9)
 	playsound(item_pawn.loc, attack_sound, 100, TRUE)
-	controller.blackboard[throw_count_key]++
+	controller.add_blackboard_key(throw_count_key, 1)
 	if(controller.blackboard[throw_count_key] >= max_attempts)
 		finish_action(controller, TRUE, target_key, throw_count_key)
 
@@ -48,8 +46,8 @@
 	reset_blackboard(controller, succeeded, target_key, throw_count_key)
 
 /datum/ai_behavior/item_move_close_and_attack/proc/reset_blackboard(datum/ai_controller/controller, succeeded, target_key, throw_count_key)
-	controller.blackboard -= target_key
-	controller.blackboard[throw_count_key] = 0
+	controller.clear_blackboard_key(target_key)
+	controller.set_blackboard_key(throw_count_key, 0)
 
 /datum/ai_behavior/item_move_close_and_attack/ghostly
 	attack_sound = 'sound/items/haunted/ghostitemattack.ogg'
@@ -58,6 +56,5 @@
 /datum/ai_behavior/item_move_close_and_attack/ghostly/haunted
 
 /datum/ai_behavior/item_move_close_and_attack/ghostly/haunted/finish_action(datum/ai_controller/controller, succeeded, target_key, throw_count_key)
-	var/datum/weakref/target_ref = controller.blackboard[target_key]
-	controller.blackboard[BB_TO_HAUNT_LIST][target_ref]--
+	controller.add_blackboard_key_assoc(BB_TO_HAUNT_LIST, controller.blackboard[target_key], -1)
 	return ..()

code/datums/ai/babies/babies_behaviors.dm

@@ -37,7 +37,7 @@
 			return
 
 	if(partner && children < max_children)
-		controller.blackboard[target_key] = WEAKREF(partner)
+		controller.set_blackboard_key(target_key, partner)
 
 	finish_action(controller, TRUE)
 
@@ -49,17 +49,15 @@
 	behavior_flags = AI_BEHAVIOR_REQUIRE_MOVEMENT
 
 /datum/ai_behavior/make_babies/setup(datum/ai_controller/controller, target_key, child_types_key)
-	var/datum/weakref/weak_target = controller.blackboard[target_key]
-	var/atom/target = weak_target?.resolve()
+	var/atom/target = controller.blackboard[target_key]
 	if(!target)
 		return FALSE
 	set_movement_target(controller, target)
 	return TRUE
 
 /datum/ai_behavior/make_babies/perform(seconds_per_tick, datum/ai_controller/controller, target_key, child_types_key)
 	. = ..()
-	var/datum/weakref/weak_target = controller.blackboard[target_key]
-	var/mob/target = weak_target?.resolve()
+	var/mob/target = controller.blackboard[target_key]
 	if(!target || target.stat != CONSCIOUS)
 		finish_action(controller, FALSE, target_key)
 		return
@@ -74,4 +72,4 @@
 /datum/ai_behavior/make_babies/finish_action(datum/ai_controller/controller, succeeded, target_key)
 	. = ..()
 
-	controller.blackboard -= target_key
+	controller.clear_blackboard_key(target_key)

code/datums/ai/babies/babies_subtrees.dm

@@ -20,15 +20,13 @@
 	if(is_type_in_list(controller.pawn, baby_types))
 		return
 
-	var/datum/weakref/weak_target = controller.blackboard[BB_BABIES_TARGET]
-	var/atom/target = weak_target?.resolve()
+	var/atom/target = controller.blackboard[BB_BABIES_TARGET]
 
 	// Find target
-	if(!target || QDELETED(target))
+	if(QDELETED(target))
 		controller.queue_behavior(/datum/ai_behavior/find_partner, BB_BABIES_TARGET, BB_BABIES_PARTNER_TYPES, BB_BABIES_CHILD_TYPES)
 		return
 
 	// Do target
-	if(target)
-		controller.queue_behavior(/datum/ai_behavior/make_babies, BB_BABIES_TARGET, BB_BABIES_CHILD_TYPES)
-		return SUBTREE_RETURN_FINISH_PLANNING
+	controller.queue_behavior(/datum/ai_behavior/make_babies, BB_BABIES_TARGET, BB_BABIES_CHILD_TYPES)
+	return SUBTREE_RETURN_FINISH_PLANNING

code/datums/ai/bane/bane_behaviors.dm

@@ -1,6 +1,6 @@
-
-/datum/ai_behavior/break_spine/bane/finish_action(datum/ai_controller/controller, succeeded)
-	var/list/bane_quotes = strings("bane.json", "bane")
-	var/mob/living/bane = controller.pawn
-	bane.say(pick(bane_quotes))
-	. = ..()
+/datum/ai_behavior/break_spine/bane/finish_action(datum/ai_controller/controller, succeeded, target_key)
+	if(succeeded)
+		var/list/bane_quotes = strings("bane.json", "bane")
+		var/mob/living/bane = controller.pawn
+		bane.say(pick(bane_quotes))
+	return ..()