From ec84f16ee475ee64ff2d4e37ab38c9c1d141a34c Mon Sep 17 00:00:00 2001 From: Charlie Nolan Date: Tue, 7 Jan 2025 15:16:57 -0800 Subject: [PATCH] Fix testing and mapload MILLA issues (#27850) --- code/controllers/subsystem/SSair.dm | 73 +++++++++++++++++++++ code/datums/helper_datums/map_template.dm | 4 +- code/modules/awaymissions/zlevel_helpers.dm | 4 +- code/tests/atmos/test_ventcrawl.dm | 6 +- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/code/controllers/subsystem/SSair.dm b/code/controllers/subsystem/SSair.dm index 58acf02432b..5cbb6256890 100644 --- a/code/controllers/subsystem/SSair.dm +++ b/code/controllers/subsystem/SSair.dm @@ -87,6 +87,7 @@ SUBSYSTEM_DEF(air) /// A list of callbacks waiting for MILLA to finish its tick and enter synchronous mode. var/list/waiting_for_sync = list() + var/list/sleepable_waiting_for_sync = list() /// The coordinates of the pressure image we're currently loading. var/pressure_x = 0 @@ -709,6 +710,9 @@ SUBSYSTEM_DEF(air) // Any proc that wants MILLA to be synchronous should not sleep. SHOULD_NOT_SLEEP(TRUE) + // Just in case someone is naughty and decides to sleep, make sure that this method runs fully anyway. + set waitfor = FALSE + if(is_synchronous) var/was_safe = SSair.in_milla_safe_code SSair.in_milla_safe_code = TRUE @@ -719,11 +723,22 @@ SUBSYSTEM_DEF(air) waiting_for_sync += CB +/// Similar to addtimer, but triggers once MILLA enters synchronous mode. This version allows for sleeping if it's absolutely necessary. +/datum/controller/subsystem/air/proc/sleepable_synchronize(datum/milla_safe/CB) + sleepable_waiting_for_sync += CB + /datum/controller/subsystem/air/proc/is_in_milla_safe_code() return in_milla_safe_code /datum/controller/subsystem/air/proc/on_milla_tick_finished() + run_sleepless_callbacks() + run_sleeping_callbacks() is_synchronous = TRUE + +/datum/controller/subsystem/air/proc/run_sleepless_callbacks() + // Just in case someone is naughty and decides to sleep, make sure that this method runs fully anyway. + set waitfor = FALSE + in_milla_safe_code = TRUE for(var/datum/milla_safe/CB as anything in waiting_for_sync) // This is one of two intended places to call this otherwise-unsafe proc. @@ -731,6 +746,14 @@ SUBSYSTEM_DEF(air) waiting_for_sync.Cut() in_milla_safe_code = FALSE +/datum/controller/subsystem/air/proc/run_sleeping_callbacks() + in_milla_safe_code = TRUE + for(var/datum/milla_safe_must_sleep/CB as anything in sleepable_waiting_for_sync) + // This is one of two intended places to call this otherwise-unsafe proc. + CB.private_unsafe_invoke() + sleepable_waiting_for_sync.Cut() + in_milla_safe_code = FALSE + /proc/milla_tick_finished() // Any proc that wants MILLA to be synchronous should not sleep. SHOULD_NOT_SLEEP(TRUE) @@ -805,6 +828,56 @@ SUBSYSTEM_DEF(air) var/datum/gas_mixture/turf_air = get_turf_air(T) turf_air.copy_from(air) +/// Create a subclass of this and implement `on_run` to manipulate tile air safely. ONLY USE THIS VERSION IF YOU CAN'T AVOID SLEEPING; it will delay atmos ticks! +/datum/milla_safe_must_sleep + var/run_args = list() + +/// All subclasses should implement this. +/datum/milla_safe_must_sleep/proc/on_run(...) + CRASH("[src.type] does not implement on_run") + +/// Call this to make the subclass run when it's safe to do so. Args will be passed to on_run. +/datum/milla_safe_must_sleep/proc/invoke_async(...) + run_args = args.Copy() + SSair.sleepable_synchronize(src) + +/// Do not call this yourself. This is what is called to run your code from a safe context. +/datum/milla_safe_must_sleep/proc/private_unsafe_invoke() + soft_assert_safe() + on_run(arglist(run_args)) + +/// Used internally to check that we're running safely, but without breaking things worse if we aren't. +/datum/milla_safe_must_sleep/proc/soft_assert_safe() + ASSERT(SSair.is_in_milla_safe_code()) + +/// Fetch the air for a turf. Only use from `on_run`. +/datum/milla_safe_must_sleep/proc/get_turf_air(turf/T) + RETURN_TYPE(/datum/gas_mixture) + soft_assert_safe() + // This is one of two intended places to call this otherwise-unsafe proc. + var/datum/gas_mixture/bound_to_turf/air = T.private_unsafe_get_air() + if(air.lastread < SSair.times_fired) + var/list/milla_tile = new/list(MILLA_TILE_SIZE) + get_tile_atmos(T, milla_tile) + air.copy_from_milla(milla_tile) + air.lastread = SSair.times_fired + air.readonly = null + air.dirty = FALSE + if(!air.synchronized) + air.synchronized = TRUE + SSair.bound_mixtures += air + return air + +/// Add air to a turf. Only use from `on_run`. +/datum/milla_safe_must_sleep/proc/add_turf_air(turf/T, datum/gas_mixture/air) + var/datum/gas_mixture/turf_air = get_turf_air(T) + turf_air.merge(air) + +/// Completely replace the air for a turf. Only use from `on_run`. +/datum/milla_safe_must_sleep/proc/set_turf_air(turf/T, datum/gas_mixture/air) + var/datum/gas_mixture/turf_air = get_turf_air(T) + turf_air.copy_from(air) + #undef SSAIR_DEFERREDPIPENETS #undef SSAIR_PIPENETS #undef SSAIR_ATMOSMACHINERY diff --git a/code/datums/helper_datums/map_template.dm b/code/datums/helper_datums/map_template.dm index db20225e0c1..08437daa609 100644 --- a/code/datums/helper_datums/map_template.dm +++ b/code/datums/helper_datums/map_template.dm @@ -62,13 +62,13 @@ stack_trace("One of the smoothing corners is bust") catch(var/exception/e) GLOB.space_manager.remove_dirt(placement.z) - var/datum/milla_safe/late_setup_level/milla = new() + var/datum/milla_safe_must_sleep/late_setup_level/milla = new() milla.invoke_async(bot_left, top_right, block(ST_bot_left, ST_top_right)) message_admins("Map template [name] threw an error while loading. Safe exit attempted, but check for errors at [ADMIN_COORDJMP(placement)].") log_admin("Map template [name] threw an error while loading. Safe exit attempted.") throw e GLOB.space_manager.remove_dirt(placement.z) - var/datum/milla_safe/late_setup_level/milla = new() + var/datum/milla_safe_must_sleep/late_setup_level/milla = new() milla.invoke_async(bot_left, top_right, block(ST_bot_left, ST_top_right)) log_game("[name] loaded at [min_x],[min_y],[placement.z]") diff --git a/code/modules/awaymissions/zlevel_helpers.dm b/code/modules/awaymissions/zlevel_helpers.dm index 1b503bafabf..749abdf47a5 100644 --- a/code/modules/awaymissions/zlevel_helpers.dm +++ b/code/modules/awaymissions/zlevel_helpers.dm @@ -1,7 +1,7 @@ -/datum/milla_safe/late_setup_level +/datum/milla_safe_must_sleep/late_setup_level // Ensures that atmos and environment are set up. -/datum/milla_safe/late_setup_level/on_run(turf/bot_left, turf/top_right, smoothTurfs) +/datum/milla_safe_must_sleep/late_setup_level/on_run(turf/bot_left, turf/top_right, smoothTurfs) var/total_timer = start_watch() var/subtimer = start_watch() if(!smoothTurfs) diff --git a/code/tests/atmos/test_ventcrawl.dm b/code/tests/atmos/test_ventcrawl.dm index c757bc9e11e..ee446d15e2e 100644 --- a/code/tests/atmos/test_ventcrawl.dm +++ b/code/tests/atmos/test_ventcrawl.dm @@ -4,9 +4,9 @@ var/obj/structure/table/table = null var/setup_complete = FALSE -/datum/milla_safe/ventcrawl_test_setup +/datum/milla_safe_must_sleep/ventcrawl_test_setup -/datum/milla_safe/ventcrawl_test_setup/on_run(datum/game_test/ventcrawl/test) +/datum/milla_safe_must_sleep/ventcrawl_test_setup/on_run(datum/game_test/ventcrawl/test) // I'm sure get_area_turfs is totally deterministic and this will never go wrong var/turf/run_loc_bottom_left = test.available_turfs[1] // This setup creates turfs that initialize themselves in MILLA on creation, which is why we need to be MILLA-safe. @@ -26,7 +26,7 @@ TEST_FAIL("Couldn't find spawned test object of type: [test_object_type].") /datum/game_test/ventcrawl/Run() - var/datum/milla_safe/ventcrawl_test_setup/milla = new() + var/datum/milla_safe_must_sleep/ventcrawl_test_setup/milla = new() milla.invoke_async(src) while(!setup_complete) sleep(world.tick_lag)