From 4a2e8bb0c410b37216a758f0aa558290133a4410 Mon Sep 17 00:00:00 2001 From: Leshana Date: Mon, 20 Apr 2020 13:06:20 -0400 Subject: [PATCH 1/3] Fix return values of CanZASPass - CanZASPass is supposed to return boolean. Nobody noticed this bug because ATMOS_PASS_YES and ATMOS_PASS_NO happen to be defined as 1 and 0. But thats not a good assumption to make, so lets fix it! --- code/game/machinery/doors/blast_door.dm | 2 +- code/game/machinery/doors/door.dm | 2 +- code/game/machinery/doors/windowdoor.dm | 4 ++-- code/game/objects/effects/zone_divider.dm | 4 ++-- code/game/objects/structures/window.dm | 4 ++-- code/modules/mob/living/living_movement.dm | 5 +++-- code/modules/power/turbine.dm | 2 +- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/code/game/machinery/doors/blast_door.dm b/code/game/machinery/doors/blast_door.dm index 36faf82b002..5ff587a7d91 100644 --- a/code/game/machinery/doors/blast_door.dm +++ b/code/game/machinery/doors/blast_door.dm @@ -269,7 +269,7 @@ // If for some reason this is actually needed for something important, uncomment this. /obj/machinery/door/blast/CanZASPass(turf/T, is_zone) if(is_zone) - return ATMOS_PASS_YES + return TRUE return ..() */ diff --git a/code/game/machinery/doors/door.dm b/code/game/machinery/doors/door.dm index 35d089513cb..0d6d798a900 100644 --- a/code/game/machinery/doors/door.dm +++ b/code/game/machinery/doors/door.dm @@ -157,7 +157,7 @@ /obj/machinery/door/CanZASPass(turf/T, is_zone) if(is_zone) - return block_air_zones ? ATMOS_PASS_NO : ATMOS_PASS_YES + return !block_air_zones return ..() /obj/machinery/door/proc/bumpopen(mob/user as mob) diff --git a/code/game/machinery/doors/windowdoor.dm b/code/game/machinery/doors/windowdoor.dm index 1396b5f5549..2414ac525b0 100644 --- a/code/game/machinery/doors/windowdoor.dm +++ b/code/game/machinery/doors/windowdoor.dm @@ -94,9 +94,9 @@ /obj/machinery/door/window/CanZASPass(turf/T, is_zone) if(get_dir(T, loc) == turn(dir, 180)) if(is_zone) // No merging allowed. - return ATMOS_PASS_NO + return FALSE return ..() // Air can flow if open (density == FALSE). - return ATMOS_PASS_YES // Windoors don't block if not facing the right way. + return TRUE // Windoors don't block if not facing the right way. /obj/machinery/door/window/CheckExit(atom/movable/mover as mob|obj, turf/target as turf) if(istype(mover) && mover.checkpass(PASSGLASS)) diff --git a/code/game/objects/effects/zone_divider.dm b/code/game/objects/effects/zone_divider.dm index 01e6834f164..90f9b0b6ff6 100644 --- a/code/game/objects/effects/zone_divider.dm +++ b/code/game/objects/effects/zone_divider.dm @@ -15,5 +15,5 @@ if(air_master && air_master.current_cycle == 0) spawn(1) air_master.mark_for_update(get_turf(src)) - return ATMOS_PASS_NO - return is_zone ? ATMOS_PASS_NO : ATMOS_PASS_YES // Anything except zones can pass + return FALSE + return is_zone ? FALSE : TRUE // Anything except zones can pass diff --git a/code/game/objects/structures/window.dm b/code/game/objects/structures/window.dm index eb64e2414d6..987b37aaf64 100644 --- a/code/game/objects/structures/window.dm +++ b/code/game/objects/structures/window.dm @@ -144,8 +144,8 @@ /obj/structure/window/CanZASPass(turf/T, is_zone) if(is_fulltile() || get_dir(T, loc) == turn(dir, 180)) // Make sure we're handling the border correctly. - return anchored ? ATMOS_PASS_NO : ATMOS_PASS_YES // If it's anchored, it'll block air. - return ATMOS_PASS_YES // Don't stop airflow from the other sides. + return !anchored // If it's anchored, it'll block air. + return TRUE // Don't stop airflow from the other sides. /obj/structure/window/CheckExit(atom/movable/O as mob|obj, target as turf) if(istype(O) && O.checkpass(PASSGLASS)) diff --git a/code/modules/mob/living/living_movement.dm b/code/modules/mob/living/living_movement.dm index f730077d706..923d9e55dd6 100644 --- a/code/modules/mob/living/living_movement.dm +++ b/code/modules/mob/living/living_movement.dm @@ -8,8 +8,9 @@ return !P.can_hit_target(src, P.permutated, src == P.original, TRUE) return (!mover.density || !density || lying) -/mob/CanZASPass(turf/T, is_zone) - return ATMOS_PASS_YES +// There is no need to override this if you're just going to unconditionally return TRUE. Set can_atmos_pass instead. +///mob/CanZASPass(turf/T, is_zone) +// return TRUE /mob/living/SelfMove(turf/n, direct) // If on walk intent, don't willingly step into hazardous tiles. diff --git a/code/modules/power/turbine.dm b/code/modules/power/turbine.dm index 29a1db0e165..5629782161c 100644 --- a/code/modules/power/turbine.dm +++ b/code/modules/power/turbine.dm @@ -96,7 +96,7 @@ // When anchored, don't let air past us. /obj/machinery/compressor/CanZASPass(turf/T, is_zone) - return anchored ? ATMOS_PASS_NO : ATMOS_PASS_YES + return !anchored /obj/machinery/compressor/proc/locate_machinery() if(turbine) From 5c752e79f4091f13ad8e76f5e33c5e3ca1684614 Mon Sep 17 00:00:00 2001 From: Leshana Date: Mon, 20 Apr 2020 13:40:34 -0400 Subject: [PATCH 2/3] Remove /turf/var/can_atmos_pass as it is unused (and redundant with /turf/var/blocks_air anyway. --- code/ZAS/Atom.dm | 2 -- 1 file changed, 2 deletions(-) diff --git a/code/ZAS/Atom.dm b/code/ZAS/Atom.dm index 0457dce1a5d..de91df365f4 100644 --- a/code/ZAS/Atom.dm +++ b/code/ZAS/Atom.dm @@ -22,8 +22,6 @@ else return can_atmos_pass -/turf/can_atmos_pass = ATMOS_PASS_NO - /turf/CanPass(atom/movable/mover, turf/target) if(!target) return FALSE From 10b0323d6f40e95edeccc6a8d5a3659015e73948 Mon Sep 17 00:00:00 2001 From: Leshana Date: Mon, 20 Apr 2020 13:37:51 -0400 Subject: [PATCH 3/3] Use can_atmos_pass to reduce proc-calls in c_airblock() - Add additional can_atmos_pass value ATMOS_PASS_PROC which indicates custom behavior requiring calling the CanZASPass proc. - The benefit being for the other three values we DON'T need to call CanZASPass at all! We already know the behavior without the overhead of a proc call. - Obviously any atom with can_atmos_pass = ATMOS_PASS_PROC cannot now call ..() in CanZASPass() since the default behavior would be to (recursively) call CanZASPass() - This required re-numbering the constants, so I also fixed all code that assumed particular values for the constants. - Switched all types which overrode CanZASPass with custom logic to be can_atmos_pass = ATMOS_PASS_PROC - Changed /turf/c_airblock() to skip calling /atom/movable/c_airblock() for the three can_atmos_pass values that don't require calling the proc. --- code/ZAS/Atom.dm | 21 +++++++++++++++++++-- code/__defines/ZAS.dm | 7 +++++++ code/__defines/atmos.dm | 6 ------ code/game/machinery/doors/door.dm | 6 +++--- code/game/machinery/doors/windowdoor.dm | 3 ++- code/game/objects/effects/zone_divider.dm | 1 + code/game/objects/structures/window.dm | 2 +- code/modules/mob/living/living_movement.dm | 4 ---- code/modules/power/turbine.dm | 1 + 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/code/ZAS/Atom.dm b/code/ZAS/Atom.dm index de91df365f4..700eded6d85 100644 --- a/code/ZAS/Atom.dm +++ b/code/ZAS/Atom.dm @@ -16,11 +16,19 @@ // Inputs: The turf the airflow is from, which may not be the same as loc. is_zone is for conditionally disallowing merging. // Outputs: Boolean if airflow can pass. /atom/proc/CanZASPass(turf/T, is_zone) + // Behaviors defined here so when people directly call c_airblock it will still obey can_atmos_pass switch(can_atmos_pass) + if(ATMOS_PASS_YES) + return TRUE + if(ATMOS_PASS_NO) + return FALSE if(ATMOS_PASS_DENSITY) return !density + if(ATMOS_PASS_PROC) + // Cowardly refuse to recursively self-call CanZASPass. The hero BYOND needs? + CRASH("can_atmos_pass = ATMOS_PASS_PROC but CanZASPass not overridden on [src] ([type])") else - return can_atmos_pass + CRASH("Invalid can_atmos_pass = [can_atmos_pass] on [src] ([type])") /turf/CanPass(atom/movable/mover, turf/target) if(!target) return FALSE @@ -87,6 +95,15 @@ turf/c_airblock(turf/other) var/result = 0 for(var/mm in contents) var/atom/movable/M = mm - result |= M.c_airblock(other) + switch(M.can_atmos_pass) + if(ATMOS_PASS_YES) + continue + if(ATMOS_PASS_NO) + return BLOCKED + if(ATMOS_PASS_DENSITY) + if(density) + return BLOCKED + if(ATMOS_PASS_PROC) + result |= M.c_airblock(other) if(result == BLOCKED) return BLOCKED return result diff --git a/code/__defines/ZAS.dm b/code/__defines/ZAS.dm index 8712dd2bf70..1760505f3a0 100644 --- a/code/__defines/ZAS.dm +++ b/code/__defines/ZAS.dm @@ -4,3 +4,10 @@ #define BLOCKED 3 // Blocked, zone boundaries will not cross even if opened. #define ZONE_MIN_SIZE 14 // Zones with less than this many turfs will always merge, even if the connection is not direct + +// Used for quickly making certain things allow airflow or not. +// More complicated, conditional airflow should override CanZASPass(). +#define ATMOS_PASS_YES 1 // Always blocks air and zones. +#define ATMOS_PASS_NO 2 // Never blocks air or zones. +#define ATMOS_PASS_DENSITY 3 // Blocks air and zones if density = 1, allows both if density = 0 +#define ATMOS_PASS_PROC 4 // Call CanZASPass() using c_airblock diff --git a/code/__defines/atmos.dm b/code/__defines/atmos.dm index 56055273e4f..424f2a26391 100644 --- a/code/__defines/atmos.dm +++ b/code/__defines/atmos.dm @@ -94,9 +94,3 @@ #define ATMOSTANK_CO2 25000 // CO2 and PH are not critically important for station, only for toxins and alternative coolants, no need to store a lot of those. #define ATMOSTANK_PHORON 25000 #define ATMOSTANK_NITROUSOXIDE 10000 // N2O doesn't have a real useful use, i guess it's on station just to allow refilling of sec's riot control canisters? - -// Used for quickly making certain things allow airflow or not. -// More complicated, conditional airflow should override CanZASPass(). -#define ATMOS_PASS_YES 1 -#define ATMOS_PASS_NO 0 -#define ATMOS_PASS_DENSITY -1 // Just checks density. \ No newline at end of file diff --git a/code/game/machinery/doors/door.dm b/code/game/machinery/doors/door.dm index 0d6d798a900..c07d17d9ac8 100644 --- a/code/game/machinery/doors/door.dm +++ b/code/game/machinery/doors/door.dm @@ -9,7 +9,7 @@ anchored = 1 opacity = 1 density = 1 - can_atmos_pass = ATMOS_PASS_DENSITY + can_atmos_pass = ATMOS_PASS_PROC layer = DOOR_OPEN_LAYER var/open_layer = DOOR_OPEN_LAYER var/closed_layer = DOOR_CLOSED_LAYER @@ -157,8 +157,8 @@ /obj/machinery/door/CanZASPass(turf/T, is_zone) if(is_zone) - return !block_air_zones - return ..() + return !block_air_zones // Block merging unless block_air_zones = 0 + return !density // Block airflow unless density = 0 /obj/machinery/door/proc/bumpopen(mob/user as mob) if(operating) return diff --git a/code/game/machinery/doors/windowdoor.dm b/code/game/machinery/doors/windowdoor.dm index 2414ac525b0..f7b8a1222de 100644 --- a/code/game/machinery/doors/windowdoor.dm +++ b/code/game/machinery/doors/windowdoor.dm @@ -14,6 +14,7 @@ opacity = 0 var/obj/item/weapon/airlock_electronics/electronics = null explosion_resistance = 5 + can_atmos_pass = ATMOS_PASS_PROC air_properties_vary_with_direction = 1 /obj/machinery/door/window/New() @@ -95,7 +96,7 @@ if(get_dir(T, loc) == turn(dir, 180)) if(is_zone) // No merging allowed. return FALSE - return ..() // Air can flow if open (density == FALSE). + return !density // Air can flow if open (density == FALSE). return TRUE // Windoors don't block if not facing the right way. /obj/machinery/door/window/CheckExit(atom/movable/mover as mob|obj, turf/target as turf) diff --git a/code/game/objects/effects/zone_divider.dm b/code/game/objects/effects/zone_divider.dm index 90f9b0b6ff6..acab441d522 100644 --- a/code/game/objects/effects/zone_divider.dm +++ b/code/game/objects/effects/zone_divider.dm @@ -7,6 +7,7 @@ anchored = 1 density = 0 opacity = 0 + can_atmos_pass = ATMOS_PASS_PROC /obj/effect/zone_divider/CanZASPass(turf/T, is_zone) // Special case to prevent us from being part of a zone during the first air master tick. diff --git a/code/game/objects/structures/window.dm b/code/game/objects/structures/window.dm index 987b37aaf64..3df62610c98 100644 --- a/code/game/objects/structures/window.dm +++ b/code/game/objects/structures/window.dm @@ -3,7 +3,7 @@ desc = "A window." icon = 'icons/obj/structures_vr.dmi' // VOREStation Edit - New icons density = 1 - can_atmos_pass = ATMOS_PASS_DENSITY + can_atmos_pass = ATMOS_PASS_PROC w_class = ITEMSIZE_NORMAL layer = WINDOW_LAYER diff --git a/code/modules/mob/living/living_movement.dm b/code/modules/mob/living/living_movement.dm index 923d9e55dd6..8256fe29a0e 100644 --- a/code/modules/mob/living/living_movement.dm +++ b/code/modules/mob/living/living_movement.dm @@ -8,10 +8,6 @@ return !P.can_hit_target(src, P.permutated, src == P.original, TRUE) return (!mover.density || !density || lying) -// There is no need to override this if you're just going to unconditionally return TRUE. Set can_atmos_pass instead. -///mob/CanZASPass(turf/T, is_zone) -// return TRUE - /mob/living/SelfMove(turf/n, direct) // If on walk intent, don't willingly step into hazardous tiles. // Unless the walker is confused. diff --git a/code/modules/power/turbine.dm b/code/modules/power/turbine.dm index 5629782161c..2caa008f0c0 100644 --- a/code/modules/power/turbine.dm +++ b/code/modules/power/turbine.dm @@ -28,6 +28,7 @@ icon_state = "compressor" anchored = TRUE density = TRUE + can_atmos_pass = ATMOS_PASS_PROC circuit = /obj/item/weapon/circuitboard/machine/power_compressor var/obj/machinery/power/turbine/turbine var/datum/gas_mixture/gas_contained