`handle_atom_del` subscription and its application to slimes

[moxian]

What Does This PR Do

Partially (but not fully) addresses #18611

Expands handle_atom_del functionality to be called on qdel not only of the atoms in your .contents but also for any arbitrary one you might be interested in and hold a ref in your vars

Sample API usage:

/obj/some/thing
  var/obj/foo/tracked_foo = null // something we care about

/obj/some/thing/proc/track_foo(obj/foo/F)
  tracked_foo = F
  subscribe_atom_del(F, "foo")  // remember to release the ref when it gets qdel'd

/obj/some/thing/proc/stop_tracking_foo()
  tracked_foo = null
  unsubscribe_atom_del(tracked_foo, "foo") // we don't care about it anymore

/obj/some/thing/handle_atom_del(atom/A)
  if(tracked_foo == A)
    tracked_foo = null  // release the ref
  ..()

Applies this new framework to xenobio slimes as a proof of concept.

Why It's Good For The Game

I tried to fix slimes gc issues by doing what i did in my couple last PRs - that is, RegisterSignal(target COMSIG_PARENT_QDELETING, .proc/on_target_delete), however that turned out to be unworkable

Here's why exactly

To be specific, here are relevant slime vars: ```dm /mob/living/simple_animal/slime var/mob/living/Target = null // AI variable - tells the slime to hunt this down varmob/living/Leader = null // AI variable - tells the slime to follow this person var/list/Friends = list() // A list of friends; they are not considered targets for feeding; passed down after splitting ```

It's fairly easy to wrap their modification into a wrapper (or just do those shenanigans in-line without a dedicated proc, but for the sake of illustration), like this:

/mob/living/simple_animal/slime/proc/set_Target(mob/living/new_target)
  src.Target = new_target
  RegisterSignal(new_target, COMSIG_PARENT_QDELETING, .proc/on_target_delete)

/mob/living/simple_animal/slime/proc/on_target_delete(atom/target)
  src.Target = null

However, Target is not the only var that can hold references, so let's add another handler for the Leader:

/mob/living/simple_animal/slime/proc/set_Leader(mob/living/new_leader)
  src.Leader = new_leader
  RegisterSignal(new_leader, COMSIG_PARENT_QDELETING, .proc/on_leader_delete)

/mob/living/simple_animal/slime/proc/on_target_delete(atom/target)
  src.Leader = null

Except that runtimes, because you cannot register more than one signal handler for each (atom, signal) pair - you can only overwrite old ones. But surely, that is surmountable? Just merge the on_delete procs into one! And indeed it sorta works:

/mob/living/simple_animal/slime/proc/set_Target(mob/living/new_target)
  src.Target = new_target
  RegisterSignal(new_target, COMSIG_PARENT_QDELETING, .proc/on_thing_delete, override=TRUE)

/mob/living/simple_animal/slime/proc/set_Leader(mob/living/new_leader)
  src.Leader = new_leader
  RegisterSignal(new_leader, COMSIG_PARENT_QDELETING, .proc/on_thing_delete, override=TRUE)

/mob/living/simple_animal/slime/proc/on_thing_delete(atom/thing)
  if(thing==src.Leader)  src.Leader = null
  if(thing==src.Target)  src.Target = null

Except the override=TRUE is kinda crude. Maybe a better idea is something like

/mob/living/simple_animal/slime/proc/set_Target(mob/living/new_target)
  maybe_start_tracking(new_target) // this MUST be before the assignment line below
  Target = new_target

/mob/living/simple_animal/slime/proc/maybe_start_tracking(atom/thing)
  if((thing == Leader) || (thing == Target) || (thing in Friends))
    return // already tracking
  RegisterSignal(thing, COMSIG_PARENT_QDELETING, .proc/on_thing_delete)

Well, that's kinda brittle but it works. Oh, but we probably want to un-register from things we are no longer interested in, now how exactly do we do that?..

/mob/living/simple_animal/slime/proc/set_Target(mob/living/new_target)
  var/old_target = Target
  Target = null
  maybe_stop_tracking(old_target ) // this MUST be after `Target = null`
  maybe_start_tracking(new_target) // this MUST be before the assignment line below
  Target = new_target

/mob/living/simple_animal/slime/proc/maybe_stop_tracking(atom/thing)
  var/refcount = 0
  if(thing == Leader)
    refcount += 1
  if(thing == Target)
    refcount += 1
  if(thing in Friends)
    refcount += 1
  if(refcount == 0)
    UnregisterSignal(thing, COMSIG_PARENT_QDELETING)

That sure sounds like fun...

But most crucially - this whole thingymajingy absolutely does not play well with inheritance. Like, how do you integrate the same sort of fix for the

/mob/living/simple_animal
  var/mob/living/carbon/human/master_commander = null //holding var for determining who own/controls a sentient simple animal (for sentience potions).

into the system? Like.. you cannot...

So i reasoned it's better to solve the issue at the framework level, and apply it to the slimes as a proof of concept that it works.

For reviewers

There are three parts to this PR:

1. code/game/atoms.dm + code/game/atoms_movable.dm , which create the framework
2. mob/living/animal/slime/*.dm, which applies that to the slimes and
3. random occurences of handle_atom_del that used to not call parent, which became an SDMM error now.

Testing

Feed slime plasma ; atomProcCall set_Leader with a mob as an argument Feed another slime plasma ; feed it monkeys, wait for it to split VV the slimes, both the originals and the newly created - see that they have me in the Friends list. gib self

Observe Leader and Friends being properly cleaned up Observe no slime-releated GC issues; more specifically:

[2022-07-29T22:43:37] Starting up. Round ID is NULL
[2022-07-29T22:43:37] -------------------------
[2022-07-29T22:52:37] Beginning search for references to a /mob/living/carbon/human/tajaran.
[2022-07-29T22:52:37] Finished searching globals
[2022-07-29T22:54:22] Finished searching atoms
[2022-07-29T22:54:31] Found /mob/living/carbon/human/tajaran [0x3000006] in /datum/mind's [0x21014985] current var. World -> /datum/mind
[2022-07-29T22:54:34] Finished searching datums
[2022-07-29T22:54:34] Finished searching clients
[2022-07-29T22:54:34] Completed search for references to a /mob/living/carbon/human/tajaran.
[2022-07-29T22:54:34] GC: -- [0x2100000d] | /mob/living/carbon/human/tajaran was unable to be GC'd --
[2022-07-29T22:58:20] Beginning search for references to a /mob/living/carbon/human/skrell.
[2022-07-29T22:58:20] Finished searching globals
[2022-07-29T23:00:08] Finished searching atoms
[2022-07-29T23:00:21] Finished searching datums
[2022-07-29T23:00:21] Finished searching clients
[2022-07-29T23:00:21] Completed search for references to a /mob/living/carbon/human/skrell.
[2022-07-29T23:00:21] GC: -- [0x2100000d] | /mob/living/carbon/human/skrell was unable to be GC'd --

Changelog

Not player-facing

[AffectedArc07]

Nice changelog

[hal9000PR]

we aren't sure this is a good idea for a couple of reasons:

1. It makes a new list on all atom/movable
2. It adds a new system for the purpose of slime GC failures when we could instead convert Target, Leader and Friends to use UIDs, thereby making this a non-issue.
3. A similar system can be baked into RegisterSignal to allow a datum to register COMSIG_PARENT_QDELETING and a proc multiple times to itself

and the contrib isnt available for review. so we are closing this PR