Files
Bubberstation/code/datums/ai/movement/ai_movement_basic_avoidance.dm
SkyratBot 7c90b0cc48 [MIRROR] Improves how ai movement checks for if movement is allowed [MDB IGNORE] (#24920)
* Improves how ai movement checks for if movement is allowed (#79620)

## About The Pull Request

While looking into the moveloop processing issues I found a place where
moveloops were being interacted with while qdeleted. This restructures
the code a bit to prevent that being possible and also improves the
design overall here so implementers of `allowed_to_move` don't need to
be careful about their return value, which is just begging for a bug
down the line. `increment_pathing_failure` also wasn't getting called
when subtypes stopped movement.

The rest of this description is a bit of an informative lecture.

There's a pattern you do on occasion when designing public facing
functions where you have a public, but non overridden, function with
some default behavior that must always run, calling on an internal
function that *is* overridden but never called from any other location.
This is a good pattern for making sure some core behavior always runs or
for making sanity checks that can't be dodged, as the internal function
never even gets called when it has been determined that something needs
to cancel the whole thing.

Previously this code had something that looked kinda like this, there
was a proc called `pre_move` that was not supposed to be overridden, and
a proc it called called `allowed_to_move`. However, contrary to the
usual reason for doing this, `pre_move` had no behavior itself and was
meaningless. `allowed_to_move` on the other hand had exactly the sort of
sanity check that we'd want using this pattern in the form of
`if(!controller.able_to_run())`. This was allowing subtypes access to a
qdeleted move loop when it had just been deleted in the parent's version
of the proc.

This is your yearly reminder to watch out for cargo culting.

* Improves how ai movement checks for if movement is allowed

---------

Co-authored-by: Emmett Gaines <ninjanomnom@gmail.com>
2023-11-11 01:00:56 -05:00

25 lines
1.2 KiB
Plaintext

///Uses Byond's basic obstacle avoidance movement
/datum/ai_movement/basic_avoidance
max_pathing_attempts = 10
/// Movement flags to pass to the loop
var/move_flags = NONE
/datum/ai_movement/basic_avoidance/start_moving_towards(datum/ai_controller/controller, atom/current_movement_target, min_distance)
. = ..()
var/atom/movable/moving = controller.pawn
var/min_dist = controller.blackboard[BB_CURRENT_MIN_MOVE_DISTANCE]
var/delay = controller.movement_delay
var/datum/move_loop/loop = SSmove_manager.move_to(moving, current_movement_target, min_dist, delay, flags = move_flags, subsystem = SSai_movement, extra_info = controller)
RegisterSignal(loop, COMSIG_MOVELOOP_PREPROCESS_CHECK, PROC_REF(pre_move))
RegisterSignal(loop, COMSIG_MOVELOOP_POSTPROCESS, PROC_REF(post_move))
/datum/ai_movement/basic_avoidance/allowed_to_move(datum/move_loop/has_target/dist_bound/source)
. = ..()
var/turf/target_turf = get_step_towards(source.moving, source.target)
if(!target_turf?.can_cross_safely(source.moving))
return FALSE
/// Move immediately and don't update our facing
/datum/ai_movement/basic_avoidance/backstep
move_flags = MOVEMENT_LOOP_START_FAST | MOVEMENT_LOOP_NO_DIR_UPDATE