[MIRROR] fixes contents not being removed from the spatial grid when deleted during movement between 2 grid cells [MDB IGNORE] (#21524)

* fixes contents not being removed from the spatial grid when deleted during movement between 2 grid cells (#75658)

## About The Pull Request
fixes the flaky test reports for cockroaches being stuck in the spatial
grid (which mothblocks seems to have closed all of)

cockroaches get deleted when they die, so theres a spurious unit test
failure where if a cockroach is on a tile in grid cell A and moves to a
lava tile in grid cell B, they will get killed when lava.Entered() is
called, then deleted, and when /atom/movable/Destroy() is called we try
to take them out of grid cell B (because their loc is the lava tile) but
they were never added to that cell yet because their movement never
finished, so that doesnt do anything. THEN moveToNullspace() is called,
that movement finishes before the first movement, and then in
Moved(old_loc = lava turf) we try to remove it from grid cell B which
again doesnt work, and then the first movements Moved(old_loc = original
turf) is called where we can actually remove them from the correct grid
cell, except we cant because in exit_cell() we subtract
`old_target.important_recursive_contents[channel]` from the cells
content lists, and since the target is deleted by this point it doesnt
have important_recursive_contents. so the fix here is changing this so
it subtracts `old_target.important_recursive_contents?[type] ||
old_target` instead, which works if the target is deleted.

also fixes some Entered() overrides that dont call parent and improves
documentation on spatial grid defines
## Why It's Good For The Game
fixes it without needing the change_loc() setter

* fixes contents not being removed from the spatial grid when deleted during movement between 2 grid cells

---------

Co-authored-by: Kylerace <kylerlumpkin1@gmail.com>
This commit is contained in:
SkyratBot
2023-05-31 13:16:41 +02:00
committed by GitHub
parent 32429145f4
commit b2a352dbf6
5 changed files with 24 additions and 22 deletions

View File

@@ -1,12 +1,13 @@
///each cell in a spatial_grid is this many turfs in length and width
/// each cell in a spatial_grid is this many turfs in length and width (with world.max(x or y) being 255, 15 of these fit on each side of a z level)
#define SPATIAL_GRID_CELLSIZE 17
///Takes a coordinate, and spits out the spatial grid index (x or y) it's inside
/// Takes a coordinate, and spits out the spatial grid index (x or y) it's inside
#define GET_SPATIAL_INDEX(coord) ROUND_UP((coord) / SPATIAL_GRID_CELLSIZE)
#define GRID_INDEX_TO_COORDS(index) (index * SPATIAL_GRID_CELLSIZE)
/// changes the cell_(x or y) vars on /datum/spatial_grid_cell to the x or y coordinate on the map for the LOWER LEFT CORNER of the grid cell.
/// index is from 1 to SPATIAL_GRID_CELLS_PER_SIDE
#define GRID_INDEX_TO_COORDS(index) ((((index) - 1) * SPATIAL_GRID_CELLSIZE) + 1)
/// number of grid cells per x or y side of all z levels. pass in world.maxx or world.maxy
#define SPATIAL_GRID_CELLS_PER_SIDE(world_bounds) GET_SPATIAL_INDEX(world_bounds)
#define SPATIAL_GRID_CHANNELS 2
//grid contents channels
///everything that is hearing sensitive is stored in this channel

View File

@@ -435,18 +435,18 @@ SUBSYSTEM_DEF(spatial_grid)
for(var/type in spatial_grid_categories[old_target.spatial_grid_key])
switch(type)
if(SPATIAL_GRID_CONTENTS_TYPE_CLIENTS)
var/list/old_target_contents = old_target.important_recursive_contents //cache for sanic speeds (lists are references anyways)
GRID_CELL_REMOVE(intersecting_cell.client_contents, old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_CLIENTS])
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(SPATIAL_GRID_CONTENTS_TYPE_CLIENTS), old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_CLIENTS])
var/list/old_target_contents = old_target.important_recursive_contents?[type] || old_target
GRID_CELL_REMOVE(intersecting_cell.client_contents, old_target_contents)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(type), old_target_contents)
if(SPATIAL_GRID_CONTENTS_TYPE_HEARING)
var/list/old_target_contents = old_target.important_recursive_contents //cache for sanic speeds (lists are references anyways)
GRID_CELL_REMOVE(intersecting_cell.hearing_contents, old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_HEARING])
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(SPATIAL_GRID_CONTENTS_TYPE_HEARING), old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_HEARING])
var/list/old_target_contents = old_target.important_recursive_contents?[type] || old_target
GRID_CELL_REMOVE(intersecting_cell.hearing_contents, old_target_contents)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(type), old_target_contents)
if(SPATIAL_GRID_CONTENTS_TYPE_ATMOS)
GRID_CELL_REMOVE(intersecting_cell.atmos_contents, old_target)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(SPATIAL_GRID_CONTENTS_TYPE_ATMOS), old_target)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(type), old_target)
return TRUE
@@ -467,14 +467,14 @@ SUBSYSTEM_DEF(spatial_grid)
switch(exclusive_type)
if(SPATIAL_GRID_CONTENTS_TYPE_CLIENTS)
var/list/old_target_contents = old_target.important_recursive_contents //cache for sanic speeds (lists are references anyways)
GRID_CELL_REMOVE(intersecting_cell.client_contents, old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_CLIENTS])
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(exclusive_type), old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_CLIENTS])
var/list/old_target_contents = old_target.important_recursive_contents?[exclusive_type] || old_target //cache for sanic speeds (lists are references anyways)
GRID_CELL_REMOVE(intersecting_cell.client_contents, old_target_contents)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(exclusive_type), old_target_contents)
if(SPATIAL_GRID_CONTENTS_TYPE_HEARING)
var/list/old_target_contents = old_target.important_recursive_contents
GRID_CELL_REMOVE(intersecting_cell.hearing_contents, old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_HEARING])
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(exclusive_type), old_target_contents[SPATIAL_GRID_CONTENTS_TYPE_HEARING])
var/list/old_target_contents = old_target.important_recursive_contents?[exclusive_type] || old_target
GRID_CELL_REMOVE(intersecting_cell.hearing_contents, old_target_contents)
SEND_SIGNAL(intersecting_cell, SPATIAL_GRID_CELL_EXITED(exclusive_type), old_target_contents)
if(SPATIAL_GRID_CONTENTS_TYPE_ATMOS)
GRID_CELL_REMOVE(intersecting_cell.atmos_contents, old_target)
@@ -565,7 +565,7 @@ SUBSYSTEM_DEF(spatial_grid)
#ifdef UNIT_TESTS
if(untracked_movable_error(to_remove))
find_hanging_cell_refs_for_movable(to_remove, remove_from_cells=TRUE)
find_hanging_cell_refs_for_movable(to_remove, remove_from_cells=FALSE) //dont remove from cells because we should be able to see 2 errors
return
#endif

View File

@@ -212,7 +212,7 @@
if(spatial_grid_key)
SSspatial_grid.force_remove_from_grid(src)
LAZYCLEARLIST(client_mobs_in_contents)
LAZYNULL(client_mobs_in_contents)
. = ..()
@@ -224,7 +224,7 @@
//This absolutely must be after moveToNullspace()
//We rely on Entered and Exited to manage this list, and the copy of this list that is on any /atom/movable "Containers"
//If we clear this before the nullspace move, a ref to this object will be hung in any of its movable containers
LAZYCLEARLIST(important_recursive_contents)
LAZYNULL(important_recursive_contents)
vis_locs = null //clears this atom out of all viscontents
@@ -762,7 +762,6 @@
pulling.move_from_pull(src, target_turf, glide_size)
check_pulling()
//glide_size strangely enough can change mid movement animation and update correctly while the animation is playing
//This means that if you don't override it late like this, it will just be set back by the movement update that's called when you move turfs.
if(glide_size_override)

View File

@@ -128,6 +128,7 @@
initial_gas_mix = AIRLESS_ATMOS
/turf/open/lava/Entered(atom/movable/arrived, atom/old_loc, list/atom/old_locs)
. = ..()
if(burn_stuff(arrived))
START_PROCESSING(SSobj, src)

View File

@@ -598,6 +598,7 @@
START_PROCESSING(SSobj, src)
/obj/structure/closet/stasis/Entered(atom/movable/arrived, atom/old_loc, list/atom/old_locs)
. = ..()
if(isliving(arrived) && holder_animal)
var/mob/living/L = arrived
L.notransform = 1