From 8b4fd0c334e7b26b2240afcff9012012ee64539f Mon Sep 17 00:00:00 2001 From: Kylerace Date: Wed, 11 Sep 2024 16:42:06 -0700 Subject: [PATCH] Revert "Refactor some usages of get_step_towards in loops to use get_steps_to instead" and subsequent fix (#86585) ## About The Pull Request This reverts commit 39e58f8b20115e1255af919b80fe1376c0907be4 (#86020), also reverts the "fix" from 8578180f8c7ef8c8745efbea3faa264c4bfc3984 (#86517 but keeping the unit test because sure). get_steps_to() pathfinds, it cannot be used in a raycasting algorithm in place of get_step_towards() (which is essentially get_step(us, get_dir(us,them))). the original pr was making every usage of can_see() and assorted procs pathfind to the target across the station instead of checking visibility in a straight line to the target, leading to lemon reporting can_see() taking up significantly more time than usual. the fix inserting CanReach() doesnt work at all, CanReach() is for checking if you can interact with an item in a possibly nested container in the same or adjacent turf, it has nothing to do with visibility on turfs. ## Why It's Good For The Game optimizes the optimization, fixes the fix --- code/__HELPERS/atoms.dm | 18 ++++++++++-------- code/__HELPERS/spatial_info.dm | 8 ++------ code/_onclick/click.dm | 15 +++++---------- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/code/__HELPERS/atoms.dm b/code/__HELPERS/atoms.dm index 646410026a0..6fc9086b81c 100644 --- a/code/__HELPERS/atoms.dm +++ b/code/__HELPERS/atoms.dm @@ -63,17 +63,19 @@ var/turf/target_turf = get_turf(target) if(get_dist(source, target) > length) return FALSE - if(current == target_turf || source.CanReach(target))//they are on the same turf or in reach, source can see the target + if(current == target_turf) return TRUE - var/list/steps = get_steps_to(source, target) - if(isnull(steps) || length(steps) > length) - return FALSE - for(var/direction in steps) - current = get_step(current, direction) - if(current == target_turf) - break + var/steps = 1 + if(current == target_turf)//they are on the same turf, source can see the target + return TRUE + current = get_step_towards(current, target_turf) + while(current != target_turf) + if(steps > length) + return FALSE if(IS_OPAQUE_TURF(current)) return FALSE + current = get_step_towards(current, target_turf) + steps++ return TRUE ///Get the cardinal direction between two atoms diff --git a/code/__HELPERS/spatial_info.dm b/code/__HELPERS/spatial_info.dm index 2a8b61bd01d..529532f50cf 100644 --- a/code/__HELPERS/spatial_info.dm +++ b/code/__HELPERS/spatial_info.dm @@ -195,12 +195,8 @@ var/turf/inbetween_turf = center_turf //this is the lowest overhead way of doing a loop in dm other than a goto. distance is guaranteed to be >= steps taken to target by this algorithm - var/list/steps = get_steps_to(inbetween_turf, target_turf) - if(isnull(steps)) - return - steps.Cut(distance + 1) - for(var/direction in steps) - inbetween_turf = get_step(inbetween_turf, direction) + for(var/step_counter in 1 to distance) + inbetween_turf = get_step_towards(inbetween_turf, target_turf) if(inbetween_turf == target_turf)//we've gotten to target's turf without returning due to turf opacity, so we must be able to see target break diff --git a/code/_onclick/click.dm b/code/_onclick/click.dm index 6696d985d0b..0c441ba928e 100644 --- a/code/_onclick/click.dm +++ b/code/_onclick/click.dm @@ -247,8 +247,7 @@ return TRUE /proc/CheckToolReach(atom/movable/here, atom/movable/there, reach) - . = FALSE - if(QDELETED(here) || QDELETED(there)) + if(!here || !there) return switch(reach) if(0) @@ -259,18 +258,14 @@ var/obj/dummy = new(get_turf(here)) dummy.pass_flags |= PASSTABLE dummy.SetInvisibility(INVISIBILITY_ABSTRACT) - var/list/steps = get_steps_to(dummy, there) - if(isnull(steps) || length(steps) > reach) // If the path is further than the reach, no way we can reach it anyways. - qdel(dummy) - return FALSE - for(var/direction in steps) - var/turf/next_step = get_step(dummy, direction) + for(var/i in 1 to reach) //Limit it to that many tries + var/turf/T = get_step(dummy, get_dir(dummy, there)) if(dummy.CanReach(there)) qdel(dummy) return TRUE - if(!dummy.Move(next_step)) // We're blocked, nope. + if(!dummy.Move(T)) //we're blocked! qdel(dummy) - return FALSE + return qdel(dummy) /// Default behavior: ignore double clicks (the second click that makes the doubleclick call already calls for a normal click)