Skip to content

Commit

Permalink
[MIRROR] Revert "Makes immerse use weakrefs", prioritizing some ref r…
Browse files Browse the repository at this point in the history
…emoval lines instead. (#504)

* Revert "Makes immerse use weakrefs", prioritizing some ref removal lines instead. (#80707)

## About The Pull Request
For a moment, I had forgot about saying I'd take a look into it, but it
seems the new fix to hard dels is causing some runtimes with empty
weakrefs. Beside, WEAKREF() doesn't work well with qdeleting atoms (so
you'd have to access the weak_reference var directly).
How immersion works is quite confusing even for me who coded it, trying
to work around some of the hefty limitations of the engine truly blows.
I could even ask MrMelbert to make a proc-chain chart for it.

But yeah, long story short, I only have a bare idea where the uncleared
refs would be. I suspect it could be `immersed_movables`. It's totally
possible since the proc can early return in a few cases, thus skipping
the ref removal, hence the title.

## Why It's Good For The Game
I didn't like the PR that implemented weakref usage into the element,
but I let it pass because "hard dels = bad". However, the runtimes
aren't that much more pleasant either.

## Changelog
N/A

* Revert "Makes immerse use weakrefs", prioritizing some ref removal lines instead.

---------

Co-authored-by: Ghom <[email protected]>
Co-authored-by: NovaBot <[email protected]>
  • Loading branch information
3 people authored and FFMirrorBot committed Jan 17, 2024
1 parent bdffc38 commit 88ffb75
Showing 1 changed file with 16 additions and 18 deletions.
34 changes: 16 additions & 18 deletions code/datums/elements/immerse.dm
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
if(!isturf(target) || !icon || !icon_state || !mask_icon)
return ELEMENT_INCOMPATIBLE

if(!movables_to_ignore)
if(isnull(movables_to_ignore))
movables_to_ignore = typecacheof(list(
/obj/effect,
/mob/dead,
Expand Down Expand Up @@ -96,8 +96,8 @@
/datum/element/immerse/proc/stop_immersion(turf/source)
SIGNAL_HANDLER
UnregisterSignal(source, list(COMSIG_ATOM_ABSTRACT_ENTERED, COMSIG_ATOM_AFTER_SUCCESSFUL_INITIALIZED_ON, COMSIG_ATOM_ABSTRACT_EXITED))
for(var/datum/weakref/movable as anything in attached_turfs_and_movables[source])
remove_from_element(source, movable.resolve())
for(var/atom/movable/movable as anything in attached_turfs_and_movables[source])
remove_from_element(source, movable)
attached_turfs_and_movables -= source

/**
Expand All @@ -122,7 +122,7 @@

try_immerse(movable, buckled)
RegisterSignal(movable, COMSIG_QDELETING, PROC_REF(on_movable_qdel))
LAZYADD(attached_turfs_and_movables[source], WEAKREF(movable))
LAZYADD(attached_turfs_and_movables[source], movable)
ADD_TRAIT(movable, TRAIT_IMMERSED, ELEMENT_TRAIT(src))

/datum/element/immerse/proc/on_movable_qdel(atom/movable/source)
Expand Down Expand Up @@ -170,7 +170,7 @@

movable.vis_contents |= vis_overlay

LAZYSET(immersed_movables, WEAKREF(movable), vis_overlay)
LAZYSET(immersed_movables, movable, vis_overlay)

///Initializes and caches a new visual overlay given parameters such as width, height and whether it should appear fully underwater.
/datum/element/immerse/proc/generate_vis_overlay(width, height, is_below_water)
Expand Down Expand Up @@ -212,16 +212,14 @@

///This proc removes the vis_overlay, the keep together trait and some signals from the movable.
/datum/element/immerse/proc/remove_immerse_overlay(atom/movable/movable)
var/atom/movable/immerse_overlay/vis_overlay = LAZYACCESS(immersed_movables, WEAKREF(movable))
if(!vis_overlay)
return
var/atom/movable/immerse_overlay/vis_overlay = LAZYACCESS(immersed_movables, movable)
LAZYREMOVE(immersed_movables, movable)
REMOVE_KEEP_TOGETHER(movable, ELEMENT_TRAIT(src))
movable.vis_contents -= vis_overlay
LAZYREMOVE(immersed_movables, WEAKREF(movable))
if(HAS_TRAIT(movable, TRAIT_UNIQUE_IMMERSE))
UnregisterSignal(movable, list(COMSIG_ATOM_SPIN_ANIMATION, COMSIG_LIVING_POST_UPDATE_TRANSFORM))
qdel(vis_overlay)
REMOVE_KEEP_TOGETHER(movable, ELEMENT_TRAIT(src))

UnregisterSignal(movable, list(COMSIG_ATOM_SPIN_ANIMATION, COMSIG_LIVING_POST_UPDATE_TRANSFORM, COMSIG_QDELETING))
if(!QDELETED(vis_overlay))
qdel(vis_overlay)
/**
* Called by init_or_entered() and on_set_buckled().
* This applies the overlay if neither the movable or whatever is buckled to (exclusive to living mobs) are flying
Expand Down Expand Up @@ -298,8 +296,8 @@
if(!(exited.loc in attached_turfs_and_movables))
remove_from_element(source, exited)
else
LAZYREMOVE(attached_turfs_and_movables[source], WEAKREF(exited))
LAZYADD(attached_turfs_and_movables[exited.loc], WEAKREF(exited))
LAZYREMOVE(attached_turfs_and_movables[source], exited)
LAZYADD(attached_turfs_and_movables[exited.loc], exited)

///Remove any signal, overlay, trait given to the movable and reference to it within the element.
/datum/element/immerse/proc/remove_from_element(turf/source, atom/movable/movable)
Expand All @@ -309,9 +307,9 @@
buckled = living_mob.buckled
try_unimmerse(movable, buckled)

LAZYREMOVE(attached_turfs_and_movables[source], movable)
UnregisterSignal(movable, list(COMSIG_LIVING_SET_BUCKLED, COMSIG_QDELETING))
REMOVE_TRAIT(movable, TRAIT_IMMERSED, ELEMENT_TRAIT(src))
LAZYREMOVE(attached_turfs_and_movables[source], WEAKREF(movable))

/// A band-aid to keep the (unique) visual overlay from scaling and rotating along with its owner. I'm sorry.
/datum/element/immerse/proc/on_update_transform(mob/living/source, resize, new_lying_angle, is_opposite_angle)
Expand All @@ -320,7 +318,7 @@
new_transform.Scale(1/source.current_size)
new_transform.Turn(-new_lying_angle)

var/atom/movable/immerse_overlay/vis_overlay = immersed_movables[WEAKREF(source)]
var/atom/movable/immerse_overlay/vis_overlay = immersed_movables[source]
if(is_opposite_angle)
vis_overlay.transform = new_transform
vis_overlay.adjust_living_overlay_offset(source)
Expand Down Expand Up @@ -361,7 +359,7 @@
///Spin the overlay in the opposite direction so it doesn't look like it's spinning at all.
/datum/element/immerse/proc/on_spin_animation(atom/source, speed, loops, segments, segment)
SIGNAL_HANDLER
var/atom/movable/immerse_overlay/vis_overlay = immersed_movables[WEAKREF(source)]
var/atom/movable/immerse_overlay/vis_overlay = immersed_movables[source]
vis_overlay.do_spin_animation(speed, loops, segments, -segment)

///We need to make sure to remove hard refs from the element when deleted.
Expand Down

0 comments on commit 88ffb75

Please sign in to comment.