From 5a496e9842f41baae6edb34a8c31de4e67497732 Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Thu, 15 Feb 2024 23:38:09 +0100 Subject: [PATCH] [MIRROR] Micro-Optimize keyLoop's self time (#26513) * Micro-Optimize keyLoop's self time (#81464) ## About The Pull Request This is a REALLY hot proc, takes up to like 2% of total cpu at highpop Let's micro it then First, clients do not go null at random. It's not predictable per say but it is consistent. We can use this understanding to remove a bunch of null checks here For loops are expensive. So rather then doing one each keyLoop, let's cache the client's intended move direction on the client. Simplifies some other code too There is no sense running a turn call if it would have no effect, let's be more intelligent about this ## Changelog :cl: refactor: Fucks with how movement keys are handled. Please report any bugs /:cl: * Micro-Optimize keyLoop's self time --------- Co-authored-by: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> --- code/datums/components/jetpack.dm | 2 +- code/datums/components/scope.dm | 2 +- code/modules/client/client_defines.dm | 3 ++ code/modules/client/client_procs.dm | 1 + code/modules/keybindings/bindings_atom.dm | 33 ++++++++++++++------- code/modules/keybindings/bindings_client.dm | 14 +++++---- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/code/datums/components/jetpack.dm b/code/datums/components/jetpack.dm index 3e0fd28ed80..437660abc82 100644 --- a/code/datums/components/jetpack.dm +++ b/code/datums/components/jetpack.dm @@ -116,7 +116,7 @@ return if(user.throwing)//You don't must use jet if you thrown return - if(length(user.client.keys_held & user.client.movement_keys))//You use jet when press keys. yes. + if(user.client.intended_direction)//You use jet when press keys. yes. thrust() /datum/component/jetpack/proc/pre_move_react(mob/user) diff --git a/code/datums/components/scope.dm b/code/datums/components/scope.dm index b413b6f8e51..531ff9e9962 100644 --- a/code/datums/components/scope.dm +++ b/code/datums/components/scope.dm @@ -60,7 +60,7 @@ stop_zooming(user_mob) return tracker.calculate_params() - if(!length(user_client.keys_held & user_client.movement_keys)) + if(!user_client.intended_direction) user_mob.face_atom(tracker.given_turf) animate(user_client, world.tick_lag, pixel_x = tracker.given_x, pixel_y = tracker.given_y) diff --git a/code/modules/client/client_defines.dm b/code/modules/client/client_defines.dm index 15a8e4454fc..45ccda8b92b 100644 --- a/code/modules/client/client_defines.dm +++ b/code/modules/client/client_defines.dm @@ -240,6 +240,9 @@ var/list/keys_held = list() /// A buffer for combinations such of modifiers + keys (ex: CtrlD, AltE, ShiftT). Format: `"key"` -> `"combo"` (ex: `"D"` -> `"CtrlD"`) var/list/key_combos_held = list() + /// The direction we WANT to move, based off our keybinds + /// Will be udpated to be the actual direction later on + var/intended_direction = NONE /* ** These next two vars are to apply movement for keypresses and releases made while move delayed. ** Because discarding that input makes the game less responsive. diff --git a/code/modules/client/client_procs.dm b/code/modules/client/client_procs.dm index 4e1847264ae..a197c5d3cdf 100644 --- a/code/modules/client/client_procs.dm +++ b/code/modules/client/client_procs.dm @@ -1044,6 +1044,7 @@ GLOBAL_LIST_INIT(blacklisted_builds, list( winset(src, "default-[REF(key)]", "parent=default;name=[key];command=[asay]") else winset(src, "default-[REF(key)]", "parent=default;name=[key];command=") + calculate_move_dir() /client/proc/change_view(new_size) if (isnull(new_size)) diff --git a/code/modules/keybindings/bindings_atom.dm b/code/modules/keybindings/bindings_atom.dm index 6dadcd5768e..e9e38489a4d 100644 --- a/code/modules/keybindings/bindings_atom.dm +++ b/code/modules/keybindings/bindings_atom.dm @@ -2,12 +2,18 @@ // Only way to do that is to tie the behavior into the focus's keyLoop(). /atom/movable/keyLoop(client/user) - var/movement_dir = NONE - for(var/_key in user?.keys_held) - movement_dir = movement_dir | user.movement_keys[_key] - if(user?.next_move_dir_add) - movement_dir |= user.next_move_dir_add - if(user?.next_move_dir_sub) + // Clients don't go null randomly. They do go null unexpectedly though, when they're poked in particular ways + // keyLoop is called by a for loop over mobs. We're guarenteed that all the mobs have clients at the START + // But the move of one mob might poke the client of another, so we do this + if(!user) + return FALSE + var/movement_dir = user.intended_direction | user.next_move_dir_add + // If we're not movin anywhere, we aren't movin anywhere + // Safe because nothing adds to movement_dir after this moment + if(!movement_dir) + return FALSE + + if(user.next_move_dir_sub) movement_dir &= ~user.next_move_dir_sub // Sanity checks in case you hold left and right and up to make sure you only go up if((movement_dir & NORTH) && (movement_dir & SOUTH)) @@ -15,14 +21,21 @@ if((movement_dir & EAST) && (movement_dir & WEST)) movement_dir &= ~(EAST|WEST) - if(user && movement_dir) //If we're not moving, don't compensate, as byond will auto-fill dir otherwise + if(user.dir != NORTH && movement_dir) //If we're not moving, don't compensate, as byond will auto-fill dir otherwise movement_dir = turn(movement_dir, -dir2angle(user.dir)) //By doing this we ensure that our input direction is offset by the client (camera) direction //turn without moving while using the movement lock key, unless something wants to ignore it and move anyway - if(user?.movement_locked && !(SEND_SIGNAL(src, COMSIG_MOVABLE_KEYBIND_FACE_DIR, movement_dir) & COMSIG_IGNORE_MOVEMENT_LOCK)) + if(user.movement_locked && !(SEND_SIGNAL(src, COMSIG_MOVABLE_KEYBIND_FACE_DIR, movement_dir) & COMSIG_IGNORE_MOVEMENT_LOCK)) keybind_face_direction(movement_dir) - else - user?.Move(get_step(src, movement_dir), movement_dir) + // Null check cause of the signal above + else if(user) + user.Move(get_step(src, movement_dir), movement_dir) return !!movement_dir //true if there was actually any player input return FALSE + +/client/proc/calculate_move_dir() + var/movement_dir = NONE + for(var/_key in keys_held) + movement_dir |= movement_keys[_key] + intended_direction = movement_dir diff --git a/code/modules/keybindings/bindings_client.dm b/code/modules/keybindings/bindings_client.dm index 0aa0fd6952e..0a8cc20b38f 100644 --- a/code/modules/keybindings/bindings_client.dm +++ b/code/modules/keybindings/bindings_client.dm @@ -47,9 +47,10 @@ //the time a key was pressed isn't actually used anywhere (as of 2019-9-10) but this allows easier access usage/checking keys_held[_key] = world.time - if(!movement_locked) - var/movement = movement_keys[_key] - if(!(next_move_dir_sub & movement)) + var/movement = movement_keys[_key] + if(movement) + calculate_move_dir() + if(!movement_locked && !(next_move_dir_sub & movement)) next_move_dir_add |= movement // Client-level keybindings are ones anyone should be able to do at any time @@ -93,9 +94,10 @@ keys_held -= _key - if(!movement_locked) - var/movement = movement_keys[_key] - if(!(next_move_dir_add & movement)) + var/movement = movement_keys[_key] + if(movement) + calculate_move_dir() + if(!movement_locked && !(next_move_dir_add & movement)) next_move_dir_sub |= movement // We don't do full key for release, because for mod keys you