refactor is_blocked_turf #29324

warriorstar-orion · 2025-05-14T17:32:29Z

What Does This PR Do

This PR pulls our implementation of /proc/is_blocked_turf more in line with /tg/'s /turf/proc/is_blocked_turf.

Why It's Good For The Game

The argument names and behavior of these two different procs is just different enough to be confusing when porting things like AI behavior subtrees, so better to avoid any confusion on how to properly handle the discrepancies.

Testing

Compiles, will request TM as well.

Declaration

I confirm that I either do not require pre-approval for this PR, or I have obtained such approval and have included a screenshot to demonstrate this below.

Changelog

NPFC

kyunkyunkyun · 2025-05-14T20:13:30Z

wonder if it can be now renamed into just is_blocked() since it's now declared for turf only, not globally
mb even isblocked()

kyunkyunkyun

you can shorten it i believe

kyunkyunkyun · 2025-05-14T20:25:36Z

code/modules/mob/living/simple_animal/hostile/megafauna/blood_drunk_miner.dm

@@ -246,7 +246,7 @@ Difficulty: Medium
 		if(get_dist(src, O) >= MINER_DASH_RANGE && turf_dist_to_target <= self_dist_to_target && !islava(O) && !ischasm(O))
 			var/valid = TRUE
 			for(var/turf/T in get_line(own_turf, O))
-				if(is_blocked_turf(T, TRUE))
+				if(T.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(T.is_blocked_turf(exclude_mobs = TRUE))

if(T.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:25:44Z

code/modules/mining/lavaland/necropolis_chests.dm

@@ -680,7 +680,7 @@
 	for(var/distance in 0 to 8)
 		var/turf/current_dash_target = dash_target
 		current_dash_target = get_step(current_dash_target, user.dir)
-		if(!is_blocked_turf(current_dash_target, TRUE))
+		if(!current_dash_target.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(!current_dash_target.is_blocked_turf(exclude_mobs = TRUE))

if(!current_dash_target.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:25:47Z

code/modules/mining/lavaland/loot/hierophant_loot.dm

@@ -240,7 +241,7 @@

 /obj/item/hierophant_club/proc/teleport_mob(turf/source, mob/M, turf/target, mob/user)
 	var/turf/turf_to_teleport_to = get_step(target, get_dir(source, M)) //get position relative to caster
-	if(!turf_to_teleport_to || is_blocked_turf(turf_to_teleport_to, TRUE))
+	if(!turf_to_teleport_to || turf_to_teleport_to.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(!turf_to_teleport_to || turf_to_teleport_to.is_blocked_turf(exclude_mobs = TRUE))

if(!turf_to_teleport_to || turf_to_teleport_to.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:25:50Z

code/modules/mining/lavaland/loot/hierophant_loot.dm

@@ -205,7 +206,7 @@
 			if(beacon)
 				beacon.icon_state = "hierophant_tele_off"
 			return
-		if(is_blocked_turf(T, TRUE))
+		if(T.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(T.is_blocked_turf(exclude_mobs = TRUE))

if(T.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:25:52Z

code/modules/mining/lavaland/loot/hierophant_loot.dm

@@ -184,7 +185,7 @@
 	if(do_after(user, 40, target = user) && user && beacon)
 		var/turf/T = get_turf(beacon)
 		var/turf/source = get_turf(user)
-		if(is_blocked_turf(T, TRUE))
+		if(T.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(T.is_blocked_turf(exclude_mobs = TRUE))

if(T.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:26:17Z

code/game/objects/items/weapons/caution.dm

@@ -82,7 +82,7 @@
 		return
 	var/turf/T = get_turf(target)

-	if(is_blocked_turf(T, TRUE)) //can't put mines on a tile that has dense stuff
+	if(T.is_blocked_turf(exclude_mobs = TRUE)) //can't put mines on a tile that has dense stuff


Suggested change

if(T.is_blocked_turf(exclude_mobs = TRUE)) //can't put mines on a tile that has dense stuff

if(T.is_blocked_turf(TRUE)) //can't put mines on a tile that has dense stuff

kyunkyunkyun · 2025-05-14T20:26:21Z

code/game/objects/effects/spawners/random/random_spawner.dm

@@ -180,7 +180,7 @@

 /obj/effect/spawner/random/proc/has_unblocked_line(destination)
 	for(var/turf/potential_blockage as anything in get_line(get_turf(src), destination))
-		if(!is_blocked_turf(potential_blockage, exclude_mobs = TRUE))
+		if(!potential_blockage.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(!potential_blockage.is_blocked_turf(exclude_mobs = TRUE))

if(!potential_blockage.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:26:34Z

code/game/atoms_movable.dm

@@ -1053,7 +1053,7 @@

 	var/has_tried_to_move = FALSE

-	if(is_blocked_turf(target_turf, TRUE, excluded_objs = list(src)))
+	if(target_turf.is_blocked_turf(exclude_mobs = TRUE, ignore_atoms = list(src)))


Suggested change

if(target_turf.is_blocked_turf(exclude_mobs = TRUE, ignore_atoms = list(src)))

if(target_turf.is_blocked_turf(TRUE, ignore_atoms = list(src)))

kyunkyunkyun · 2025-05-14T20:26:38Z

code/datums/spells/ethereal_jaunt.dm

@@ -61,7 +61,7 @@
 	sleep(jaunt_in_time)
 	qdel(holder)
 	if(!QDELETED(target))
-		if(is_blocked_turf(mobloc, TRUE))
+		if(mobloc.is_blocked_turf(exclude_mobs = TRUE))


Suggested change

if(mobloc.is_blocked_turf(exclude_mobs = TRUE))

if(mobloc.is_blocked_turf(TRUE))

kyunkyunkyun · 2025-05-14T20:26:44Z

code/datums/components/shelved.dm

 		for(var/turf/potential_blockage as anything in get_line(get_turf(structure_parent), turf_in_view))
-			if(!is_blocked_turf(potential_blockage, exclude_mobs = TRUE, excluded_objs = list(parent)))
-				nearby_empty_tiles += turf_in_view
+			if(potential_blockage.is_blocked_turf(exclude_mobs = TRUE, source_atom = parent))


Suggested change

if(potential_blockage.is_blocked_turf(exclude_mobs = TRUE, source_atom = parent))

if(potential_blockage.is_blocked_turf(TRUE, parent))

I'd personally lean towards leaving these args in place, it never hurts to be a little more verbose about what the arguments to a proc are

lewcc

Changes look good to me. I'd argue that keeping the arguments for the proc is probably a net positive

warriorstar-orion · 2025-05-15T18:16:44Z

wonder if it can be now renamed into just is_blocked() since it's now declared for turf only, not globally mb even isblocked()

is_blocked() is the better name but I'm keeping it as is_blocked_turf() in order to avoid people porting AI behaviors and the like from /tg/ to have to change it.

Regarding the use of the argument name in the proc call, as @lewcc said, keeping the argument name makes it clearer what the purpose of the boolean value is for. is_blocked_turf(exclude_mobs = TRUE) is more readable than is_blocked_turf(TRUE) if you're reading through the code.

refactor is_blocked_turf

5539a7d

ParadiseSS13-Bot added Testmerge Requested This PR has a pending testmerge request -Status: Awaiting review This PR is awaiting review from the review team labels May 14, 2025

kyunkyunkyun suggested changes May 14, 2025

View reviewed changes

lewcc approved these changes May 15, 2025

View reviewed changes

ParadiseSS13-Bot added -Status: Awaiting merge This PR is ready for merge and removed -Status: Awaiting review This PR is awaiting review from the review team labels May 15, 2025

ParadiseSS13-Bot added the Testmerge Active This PR is currently testmerged on production label May 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor is_blocked_turf #29324

refactor is_blocked_turf #29324

warriorstar-orion commented May 14, 2025

kyunkyunkyun commented May 14, 2025 •

edited

Loading

kyunkyunkyun left a comment

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

kyunkyunkyun May 14, 2025

lewcc May 15, 2025

lewcc left a comment

warriorstar-orion commented May 15, 2025

	if(T.is_blocked_turf(exclude_mobs = TRUE))
	if(T.is_blocked_turf(TRUE))

	if(!current_dash_target.is_blocked_turf(exclude_mobs = TRUE))
	if(!current_dash_target.is_blocked_turf(TRUE))

	if(!turf_to_teleport_to \|\| turf_to_teleport_to.is_blocked_turf(exclude_mobs = TRUE))
	if(!turf_to_teleport_to \|\| turf_to_teleport_to.is_blocked_turf(TRUE))

	if(T.is_blocked_turf(exclude_mobs = TRUE)) //can't put mines on a tile that has dense stuff
	if(T.is_blocked_turf(TRUE)) //can't put mines on a tile that has dense stuff

	if(!potential_blockage.is_blocked_turf(exclude_mobs = TRUE))
	if(!potential_blockage.is_blocked_turf(TRUE))

	if(target_turf.is_blocked_turf(exclude_mobs = TRUE, ignore_atoms = list(src)))
	if(target_turf.is_blocked_turf(TRUE, ignore_atoms = list(src)))

	if(mobloc.is_blocked_turf(exclude_mobs = TRUE))
	if(mobloc.is_blocked_turf(TRUE))

	if(potential_blockage.is_blocked_turf(exclude_mobs = TRUE, source_atom = parent))
	if(potential_blockage.is_blocked_turf(TRUE, parent))

refactor is_blocked_turf #29324

Are you sure you want to change the base?

refactor is_blocked_turf #29324

Conversation

warriorstar-orion commented May 14, 2025

What Does This PR Do

Why It's Good For The Game

Testing

Declaration

Changelog

kyunkyunkyun commented May 14, 2025 • edited Loading

kyunkyunkyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewcc left a comment

Choose a reason for hiding this comment

warriorstar-orion commented May 15, 2025

kyunkyunkyun commented May 14, 2025 •

edited

Loading