Ports 2 PRs related to client movement: tg#8146, tg#85379 (#1051)

* Ports 2 PRs related to client movement
tgstation/tgstation#81464
tgstation/tgstation#85379

Co-authored-by: LemonInTheDark <[email protected]>
Co-authored-by: SmArtKar <[email protected]>

* Changelogs

* Fixes locked directional movement
ports tgstation/tgstation#81505
fixes #1064

---------

Co-authored-by: LemonInTheDark <[email protected]>
Co-authored-by: SmArtKar <[email protected]>

Author: 3 people

code/modules/client/client_defines.dm

@@ -247,6 +247,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 updated 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.

code/modules/client/client_procs.dm

@@ -1170,6 +1170,7 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
 					winset(src, "default-[REF(key)]", "parent=default;name=[key];command=looc")
 				if("Me")
 					winset(src, "default-[REF(key)]", "parent=default;name=[key];command=me")
+	calculate_move_dir()
 
 /client/proc/change_view(new_size)
 	if (isnull(new_size))

code/modules/keybindings/bindings_atom.dm

@@ -2,26 +2,41 @@
 // 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)
+		// No input == our removal would have done nothing
+		// So we can safely forget about it
+		user.next_move_dir_sub = NONE
+		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))
 		movement_dir &= ~(NORTH|SOUTH)
 	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
 
-	if(user?.movement_locked)
+	if(user.movement_locked)
 		keybind_face_direction(movement_dir)
 	else
-		user?.Move(get_step(src, movement_dir), movement_dir)
+		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

code/modules/keybindings/bindings_client.dm

@@ -44,9 +44,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
@@ -98,9 +99,10 @@
 	if(update_pointer == TRUE)
 		mob.update_mouse_pointer()
 
-	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

code/modules/mob/living/living.dm

@@ -116,13 +116,9 @@
 	if(now_pushing)
 		return TRUE
 
-	var/they_can_move = TRUE
-	var/their_combat_mode = FALSE
 
 	if(isliving(M))
 		var/mob/living/L = M
-		their_combat_mode = L.combat_mode
-		they_can_move = L.mobility_flags & MOBILITY_MOVE
 		//Also spread diseases
 		for(var/thing in diseases)
 			var/datum/disease/D = thing
@@ -154,23 +150,7 @@
 		return TRUE
 
 	if(!M.buckled && !M.has_buckled_mobs())
-		var/mob_swap = FALSE
-		var/too_strong = (M.move_resist > move_force) //can't swap with immovable objects unless they help us
-		if(!they_can_move) //we have to physically move them
-			if(!too_strong)
-				mob_swap = TRUE
-		else
-			//You can swap with the person you are dragging on grab intent, and restrained people in most cases
-			if(is_grabbing(M) && !too_strong)
-				mob_swap = TRUE
-			else if(
-				!(HAS_TRAIT(M, TRAIT_NOMOBSWAP) || HAS_TRAIT(src, TRAIT_NOMOBSWAP))&&\
-				((HAS_TRAIT(M, TRAIT_ARMS_RESTRAINED) && !too_strong) || !their_combat_mode) &&\
-				(HAS_TRAIT(src, TRAIT_ARMS_RESTRAINED) || !combat_mode)
-			)
-				mob_swap = TRUE
-
-		if(mob_swap)
+		if(can_mobswap_with(M))
 			//switch our position with M
 			if(loc && !loc.MultiZAdjacent(M.loc))
 				return TRUE
@@ -213,6 +193,46 @@
 			if(I.try_block_attack(M, src, "the push", 0, LEAP_ATTACK)) //close enough?
 				return TRUE
 
+/mob/living/proc/can_mobswap_with(mob/other)
+	if (HAS_TRAIT(other, TRAIT_NOMOBSWAP) || HAS_TRAIT(src, TRAIT_NOMOBSWAP))
+		return FALSE
+
+	var/they_can_move = TRUE
+	var/their_combat_mode = FALSE
+
+	if(isliving(other))
+		var/mob/living/other_living = other
+		their_combat_mode = other_living.combat_mode
+		they_can_move = other_living.mobility_flags & MOBILITY_MOVE
+
+	var/too_strong = other.move_resist > move_force
+
+	// They cannot move, see if we can push through them
+	if (!they_can_move)
+		return !too_strong
+
+	// We are pulling them and can move through
+	if (is_grabbing(other) && !too_strong)
+		return TRUE
+
+	// If we're in combat mode and not restrained we don't try to pass through people
+	if (combat_mode && !HAS_TRAIT(src, TRAIT_ARMS_RESTRAINED))
+		return FALSE
+
+	// Nor can we pass through non-restrained people in combat mode (or if they're restrained but still too strong for us)
+	if (their_combat_mode && (!HAS_TRAIT(other, TRAIT_ARMS_RESTRAINED) || too_strong))
+		return FALSE
+
+	if (isnull(other.client) || isnull(client))
+		return TRUE
+
+	// If both of us are trying to move in the same direction, let the fastest one through first
+	if (client.intended_direction == other.client.intended_direction)
+		return movement_delay < other.movement_delay
+
+	// Else, sure, let us pass
+	return TRUE
+
 /mob/living/get_photo_description(obj/item/camera/camera)
 	var/list/mob_details = list()
 	var/list/holding = list()

code/modules/mob/mob_movement.dm

@@ -66,8 +66,8 @@
 /client/Move(new_loc, direct)
 	if(world.time < move_delay) //do not move anything ahead of this check please
 		return FALSE
-	next_move_dir_add = 0
-	next_move_dir_sub = 0
+	next_move_dir_add = NONE
+	next_move_dir_sub = NONE
 	var/old_move_delay = move_delay
 	move_delay = world.time + world.tick_lag //this is here because Move() can now be called mutiple times per tick
 	if(!direct || !new_loc)

html/changelogs/AutoChangeLog-pr-81464.yml

@@ -0,0 +1,4 @@
+author: "LemonInTheDark"
+delete-after: True
+changes:
+  - refactor: "Fucks with how movement keys are handled. Please report any bugs"

html/changelogs/AutoChangeLog-pr-85379.yml

@@ -0,0 +1,5 @@
+author: "SmArtKar"
+delete-after: True
+changes:
+  - qol: "You no longer goofily swap with others trying to move in the same direction as you if you're not faster than them"
+  - code_imp: "Moved mobswap check logic into a separate proc and made it more readable"