First time making a movement system, would like some feedback on how to improve it

Help and Feedback Scripting Support
, ,
#1

I know it’s no where near to perfect, but I’d like some tips on how to polish it with the state it’s in before I start attempting to add more advanced movement features (dash, slide, etc)
Any critique, help, and advice is appreciated!
Some known bugs I haven’t been able to squash:

  1. When running (left shift) and holding down a movement button (w,a,s,d), if you stop pressing left shift while continuing to hold down a movement, it will not play the walk animation.
local player = game:GetService("Players").LocalPlayer
local humanoid = player.CharacterAdded:Wait().Humanoid
local camera = game:GetService("Workspace"):WaitForChild("Camera")
local originalWalkSpeed = 16

local UIS = game:GetService("UserInputService")
local TWS = game:GetService('TweenService')
local RS = game:GetService("RunService")

local fov = 70
local info = TweenInfo.new(.3, Enum.EasingStyle.Exponential, Enum.EasingDirection.Out)
local goal = {FieldOfView = fov + 20}
local goal2 = {FieldOfView = fov}
local Runtween = TWS:Create(game.Workspace.CurrentCamera, info, goal)
local Runtween2 = TWS:Create(game.Workspace.CurrentCamera, info, goal2)

local runAnim = script.runAnimation
local runAnimtrack = humanoid:LoadAnimation(runAnim)

local running = false
local walking = false
local crouching = false
local crouchwalk = false
local runningInputended = false

local walkAnim = script.walkAnimation
local walkAnimtrack = humanoid:LoadAnimation(walkAnim)

local jumpAnim = script.jumpAnimation
local jumpAnimtrack = humanoid:LoadAnimation(jumpAnim)

local jumprunAnim = script.jumprunAnimation
local jumprunAnimtrack = humanoid:LoadAnimation(jumprunAnim)

local crouchAnim = script.crouchAnimation --the animation for crouching
local crouchAnimtrack = humanoid:LoadAnimation(crouchAnim)

local crouchingAnim = script.crouchingAnimation --the last frame for crouching looped to hold the crouch
local crouchingAnimtrack = humanoid:LoadAnimation(crouchingAnim)

local crouchwalkAnim = script.crouchwalkAnimation --walking while crouched animation
local crouchwalkAnimtrack = humanoid:LoadAnimation(crouchwalkAnim)

--local freefallAnim = script.freefallAnimation
--local freefallAnimtrack = humanoid:LoadAnimation(freefallAnim)

--keybind detection
UIS.InputBegan:Connect(function(IO, proc)
	if proc == false and IO.KeyCode == Enum.KeyCode.LeftShift and player.Character:WaitForChild("HumanoidRootPart").Velocity.Magnitude > 1 and not running and not crouching then
		running = true
		Runtween:Play()
		humanoid.WalkSpeed = 32
		walkAnimtrack:Stop()
		crouchAnimtrack:Stop()
		crouchingAnimtrack:Stop()
		crouchAnimtrack:Stop()
		runAnimtrack:Play()
	elseif proc == false and IO.KeyCode == Enum.KeyCode.C and not crouching and not running then
		crouching = true
		humanoid.WalkSpeed = 8
		print(humanoid.WalkSpeed)
		walkAnimtrack:Stop()
		runAnimtrack:Stop() 
		crouchAnimtrack:Play()
		task.wait(0.2)
		crouchingAnimtrack:Play()
	end
end)


UIS.InputEnded:Connect(function(IO, proc)
	if proc == false and IO.KeyCode == Enum.KeyCode.LeftShift then
		runningInputended = true
	elseif proc == false and player.Character:WaitForChild("HumanoidRootPart").Velocity.Magnitude < 1 and IO.KeyCode ~= Enum.KeyCode.LeftShift then
		print("hi")
	elseif proc == false and IO.KeyCode == Enum.KeyCode.C then
		crouching = false
		humanoid.WalkSpeed = 16
		crouchwalkAnimtrack:Stop()
		crouchingAnimtrack:Stop()
		crouchAnimtrack:Stop()
	end
end)

function checkRunning()
	--run
	if running and player.Character:WaitForChild("HumanoidRootPart").Velocity.Magnitude < 1 or runningInputended or crouching then
		runningInputended = false
		running = false
		Runtween2:Play()
		humanoid.WalkSpeed = 16
		runAnimtrack:Stop()
	end

	local magnitude = humanoid.MoveDirection.Magnitude
	--walk
	if not running and magnitude > 0 and not walking and not crouching then
		walking = true
		walkAnimtrack:Play()
	elseif walking and magnitude == 0 then
		-- Stop walking if the magnitude is 0
		walking = false
		walkAnimtrack:Stop()
	end	
	--crouch	
	if crouching then
		if magnitude > 0 and not crouchwalk then
			crouchwalk = true
			crouchAnimtrack:Stop()
			crouchingAnimtrack:Stop()
			crouchwalkAnimtrack:Play()
		elseif magnitude == 0 then
			crouchwalk = false
			crouchwalkAnimtrack:Stop()
			crouchingAnimtrack:Play()
		end	
	end
end

RS.RenderStepped:Connect(checkRunning)

if not running then
	if player.Character:WaitForChild("HumanoidRootPart").Velocity.Magnitude > 1 then
		if not walkAnimtrack.IsPlaying then
			walkAnimtrack:Play()
		end
	elseif walkAnimtrack.IsPlaying then
		walkAnimtrack:Stop()
	end
end

function checkIdle()
	if humanoid.MoveDirection.Magnitude > 0 then
		print("player is moving")
	else
		print("idle")
	end
end


--jump detection
humanoid.StateChanged:Connect(function(oldState, newState)
	if newState == Enum.HumanoidStateType.Freefall and running then
		crouchingAnimtrack:Stop()
		crouchAnimtrack:Stop()
		jumprunAnimtrack:Play()
	elseif oldState == Enum.HumanoidStateType.Freefall and running then
		runAnimtrack:Play()
	elseif newState == Enum.HumanoidStateType.Freefall and not running then
		crouchingAnimtrack:Stop()
		crouchAnimtrack:Stop()
		jumpAnimtrack:Play()
	end
end)

Edit: If it’s helpful, I could showcase the animations, feel free to ask.

#2

Just on a glance, there are some issues with readability.

You should use a consistent format for naming variables, you use Runtween, crouchwalk and walkAnim all without consistent capitalization. Consider using pascal case so that you don’t accidentally mistype a variable.

Furthermore, you should avoid using acronyms like RS for RunService. It’s not immediately obvious what RS is unless you look at the beginning of the script, wasting time and increasing the chance of a mistake being made.

1 Like
#3

Thanks, I’ll work on that. I haven’t collaborated on scripting before, so my code has gotten a bit sloppy over the months. I initially attempted camelCase, but will look into pascalcase as an alternative.

#4

Your system is off to a decent start, I would recommend trying to look into some of these things
as long as you’re comfortable or wanting to explore new methods:
(This is some stuff that IMO would help with making it easier to expand with more ease)

Animation wise, as far as Run, Walk and possibly crouch, you can try to work out a system that Roblox’s provided animation system has which connects to the Humanoid.Running event which fires when the user’s speed changes. Although this would cause your animation to rely on speed instead of state. I would also like to mention there is an event which is related to Humanoid.FreeFalling, because you’re currently watching onto any state being changed just to check if the user is free-falling while running or whatnot and thought you could probably be listening onto this one instead.

Personally, I find keeping track of the state with booleans such as: running, walking, crouching kind of unintuitive, so instead of keeping track of a single type of state, you’re now relying on setting each one of these values; I would recommend creating your own Enums for state tracking:

-- // Similar to what I have
local enums = {
	Running = "Movement.Default.Running",
	Walking = "Movement.Default.Walking",
	CrouchRunning = "Movement.Crouching.Running",
	Idle = "Movement.Default.Idle",
	-- // ETC
}

local current_state = enums.Idle

Along with this, keeping track of what state ties into what animation could also possibly help,
this way you’re not exactly cancelling or stopping all animations, similar to how you deal with whether a player is idling while crouching and not, you write knowing that they’re in one of two states.

Some things I’ve noticed and aren’t exactly clear:

I’ve noticed this bit of code which doesn’t initially make sense, because it’s outside of a function:

if not running then
	if player.Character:WaitForChild("HumanoidRootPart").Velocity.Magnitude > 1 then
		if not walkAnimtrack.IsPlaying then
			walkAnimtrack:Play()
		end
	elseif walkAnimtrack.IsPlaying then
		walkAnimtrack:Stop()
	end
end

I’d like to mention that each individual state you’re creating such as Running, Walking, Crouching don’t have their own distinct method place/function which could make it confusing to read and knowing where the state is being actuated or changed. Changing it to have their own places might help with readability and future abstraction or editing in general to be simplified.

Also, for stuff such as walk speed being changed, I highly recommend using CONSTANTS at the top, so that it’s much easier to change and reuse that same value later down the line.

1 Like
#5

Why arent you posting in creations feedback or code review?

#6

I thought it would be more fitting here due to the fact that my system isn’t finished and has some major bugs.

#7

Thank you very much
Multiple people have mentioned the readability, so I’ll work on that and make a new post once I get it fixed. I’ll try those enum states too, that’s a really smart way to go about it.

1 Like