From f240be47ffb0e2b0df16a62551fa2bd1017e9adb Mon Sep 17 00:00:00 2001 From: Kylerace Date: Sat, 6 Aug 2022 16:41:15 -0700 Subject: [PATCH] fixes verbs not actually queuing. (#68990) thanks to Vallat for pointing this out whoops turns out most verbs havent been queued since may 11th because I made /datum/controller/subsystem/verb_manager have the SS_NO_INIT flag, without also removing a check in verb_manager/proc/can_queue_verb() that stops the verb callback from being queued if the subsystem isnt initialized yet. since subsystems with SS_NO_INIT obviously never have initialized set to TRUE, this always failed for every verb manager subsystem except for SSinput (because it doesnt have SS_NO_INIT). also adds a debug var to force a subsystem to always queue incoming verbs if possible. now the default verb management subsystem, and speech_controller will successfully queue verbs again. SSinput always queued verbs so that shouldnt change. --- code/__DEFINES/MC.dm | 2 +- code/controllers/subsystem/input.dm | 20 +++++----- code/controllers/subsystem/verb_manager.dm | 45 ++++++++++++++++++---- code/modules/unit_tests/resist.dm | 9 +++++ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/code/__DEFINES/MC.dm b/code/__DEFINES/MC.dm index a839b7aa754..b7468da8333 100644 --- a/code/__DEFINES/MC.dm +++ b/code/__DEFINES/MC.dm @@ -20,7 +20,7 @@ ///creates a running average of "things elapsed" per time period when you need to count via a smaller time period. ///eg you want an average number of things happening per second but you measure the event every tick (50 milliseconds). ///make sure both time intervals are in the same units. doesnt work if current_duration > total_duration or if total_duration == 0 -#define MC_AVG_OVER_TIME(average, current, total_duration, current_duration) (((total_duration - current_duration) / (total_duration)) * (average) + (current)) +#define MC_AVG_OVER_TIME(average, current, total_duration, current_duration) ((((total_duration) - (current_duration)) / (total_duration)) * (average) + (current)) #define MC_AVG_MINUTES(average, current, current_duration) (MC_AVG_OVER_TIME(average, current, 1 MINUTES, current_duration)) diff --git a/code/controllers/subsystem/input.dm b/code/controllers/subsystem/input.dm index d5ef7a514df..35930860b7f 100644 --- a/code/controllers/subsystem/input.dm +++ b/code/controllers/subsystem/input.dm @@ -65,9 +65,16 @@ VERB_MANAGER_SUBSYSTEM_DEF(input) if(usr) Click(location, control, params) - /datum/controller/subsystem/verb_manager/input/fire() + ..() + var/moves_this_run = 0 + for(var/mob/user in GLOB.keyloop_list) + moves_this_run += user.focus?.keyLoop(user.client)//only increments if a player moves due to their own input + + movements_per_second = MC_AVG_SECONDS(movements_per_second, moves_this_run, wait TICKS) + +/datum/controller/subsystem/verb_manager/input/run_verb_queue() var/deferred_clicks_this_run = 0 //acts like current_clicks but doesnt count clicks that dont get processed by SSinput for(var/datum/callback/verb_callback/queued_click as anything in verb_queue) @@ -83,16 +90,11 @@ VERB_MANAGER_SUBSYSTEM_DEF(input) verb_queue.Cut() //is ran all the way through every run, no exceptions - for(var/mob/user in GLOB.keyloop_list) - moves_this_run += user.focus?.keyLoop(user.client)//only increments if a player changes their movement input from the last tick - - clicks_per_second = MC_AVG_SECONDS(clicks_per_second, current_clicks, wait TICKS) - delayed_clicks_per_second = MC_AVG_SECONDS(delayed_clicks_per_second, deferred_clicks_this_run, wait TICKS) - movements_per_second = MC_AVG_SECONDS(movements_per_second, moves_this_run, wait TICKS) - + clicks_per_second = MC_AVG_SECONDS(clicks_per_second, current_clicks, wait SECONDS) + delayed_clicks_per_second = MC_AVG_SECONDS(delayed_clicks_per_second, deferred_clicks_this_run, wait SECONDS) current_clicks = 0 /datum/controller/subsystem/verb_manager/input/stat_entry(msg) . = ..() - . += "M/S:[round(movements_per_second,0.01)] | C/S:[round(clicks_per_second,0.01)]([round(delayed_clicks_per_second,0.01)] | CD: [round(average_click_delay,0.01)])" + . += "M/S:[round(movements_per_second,0.01)] | C/S:[round(clicks_per_second,0.01)] ([round(delayed_clicks_per_second,0.01)] | CD: [round(average_click_delay,0.01)])" diff --git a/code/controllers/subsystem/verb_manager.dm b/code/controllers/subsystem/verb_manager.dm index ecef6acc851..2095b57dcd4 100644 --- a/code/controllers/subsystem/verb_manager.dm +++ b/code/controllers/subsystem/verb_manager.dm @@ -46,6 +46,9 @@ SUBSYSTEM_DEF(verb_manager) ///for obvious reasons dont make this be TRUE on the code level this is for admins to turn on var/message_admins_on_queue = FALSE + ///always queue if possible. overides can_queue_admin_verbs but not FOR_ADMINS_IF_VERBS_FUCKED_immediately_execute_all_verbs + var/always_queue = FALSE + /** * queue a callback for the given verb/verblike proc and any given arguments to the specified verb subsystem, so that they process in the next tick. * intended to only work with verbs or verblike procs called directly from client input, use as part of TRY_QUEUE_VERB() and co. @@ -53,14 +56,30 @@ SUBSYSTEM_DEF(verb_manager) * returns TRUE if the queuing was successful, FALSE otherwise. */ /proc/_queue_verb(datum/callback/verb_callback/incoming_callback, tick_check, datum/controller/subsystem/verb_manager/subsystem_to_use = SSverb_manager, ...) - if(TICK_USAGE < tick_check \ - || QDELETED(incoming_callback) \ - || QDELETED(incoming_callback.object) \ - || !ismob(usr) \ - || QDELING(usr)) + if(QDELETED(incoming_callback) \ + || QDELETED(incoming_callback.object)) + stack_trace("_queue_verb() returned false because it was given an invalid callback!") + return FALSE + + //we want unit tests to be able to directly call verbs that attempt to queue, and since unit tests should test internal behavior, we want the queue + //to happen as if it was actually from player input if its called on a mob. +#ifdef UNIT_TESTS + if(QDELETED(usr) && ismob(incoming_callback.object)) + incoming_callback.user = WEAKREF(incoming_callback.object) + var/datum/callback/new_us = CALLBACK(arglist(list(GLOBAL_PROC, /proc/_queue_verb) + args.Copy())) + return world.push_usr(incoming_callback.object, new_us) +#endif + + //debatable whether this is needed, this is just to try and ensure that you dont use this to queue stuff that isnt from player input. + if(QDELETED(usr)) + stack_trace("_queue_verb() returned false because it wasnt called from player input!") return FALSE if(!istype(subsystem_to_use)) + stack_trace("_queue_verb() returned false because it was given an invalid subsystem to queue for!") + return FALSE + + if((TICK_USAGE < tick_check) && !subsystem_to_use.always_queue) return FALSE var/list/args_to_check = args.Copy() @@ -80,9 +99,12 @@ SUBSYSTEM_DEF(verb_manager) * in TRY_QUEUE_VERB() and co. */ /datum/controller/subsystem/verb_manager/proc/can_queue_verb(datum/callback/verb_callback/incoming_callback) - if(usr.client?.holder && !can_queue_admin_verbs \ + if(always_queue && !FOR_ADMINS_IF_VERBS_FUCKED_immediately_execute_all_verbs) + return TRUE + + if((usr.client?.holder && !can_queue_admin_verbs) \ + || (!initialized && !(flags & SS_NO_INIT)) \ || FOR_ADMINS_IF_VERBS_FUCKED_immediately_execute_all_verbs \ - || !initialized \ || !(runlevels & Master.current_runlevel)) return FALSE @@ -102,6 +124,12 @@ SUBSYSTEM_DEF(verb_manager) return TRUE /datum/controller/subsystem/verb_manager/fire(resumed) + run_verb_queue() + +/// runs through all of this subsystems queue of verb callbacks. +/// goes through the entire verb queue without yielding. +/// used so you can flush the queue outside of fire() without interfering with anything else subtype subsystems might do in fire(). +/datum/controller/subsystem/verb_manager/proc/run_verb_queue() var/executed_verbs = 0 for(var/datum/callback/verb_callback/verb_callback as anything in verb_queue) @@ -113,7 +141,8 @@ SUBSYSTEM_DEF(verb_manager) executed_verbs++ verb_queue.Cut() - verbs_executed_per_second = MC_AVG_SECONDS(verbs_executed_per_second, executed_verbs, wait TICKS) + verbs_executed_per_second = MC_AVG_SECONDS(verbs_executed_per_second, executed_verbs, wait SECONDS) + //note that wait SECONDS is incorrect if this is called outside of fire() but because byond is garbage i need to add a timer to rustg to find a valid solution /datum/controller/subsystem/verb_manager/stat_entry(msg) . = ..() diff --git a/code/modules/unit_tests/resist.dm b/code/modules/unit_tests/resist.dm index 28f6ea88011..b0acc9c8c87 100644 --- a/code/modules/unit_tests/resist.dm +++ b/code/modules/unit_tests/resist.dm @@ -12,6 +12,10 @@ // Stop, drop, and roll has a sleep call. This would delay the test, and is not necessary. call_async(human, /mob/living/verb/resist) + //since resist() is a verb that possibly queues its actual execution for the next tick, we need to make the subsystem that handles the delayed execution process + //the callback. either that or sleep ourselves and see if it ran. + SSverb_manager.run_verb_queue() + TEST_ASSERT(human.fire_stacks < 5, "Human did not lower fire stacks after resisting") /// Test that you can resist out of a container @@ -26,4 +30,9 @@ TEST_ASSERT(human in closet.contents, "Human was not in the contents of the closed closet") human.resist() + + //since resist() is a verb that possibly queues itself for the next tick, we need to make the subsystem that handles the delayed execution process + //the callback. either that or sleep ourselves and see if it ran. + SSverb_manager.run_verb_queue() + TEST_ASSERT(!(human in closet.contents), "Human resisted out of a standard closet, but was still in it")