Reviewing and improving my movement modules

Hedreon · April 12, 2024, 1:41pm

(This is my first time posting in the “Code Review” category and also my first time making a topic in the DevForum after a very long, long time)

What does the code do and what are you not satisfied with?

What the following code at the bottom of the post does is, for one, the main focus of this topic, the crawl module, it adds crawling functionality and the run module adds sprinting functionality with FOV zooming and both the crawl module and the run module communicate together through attributes (I don’t know if it’s safe/secure enough) so that sprinting functionality could still exist in a realistic way when the player is crawling. The helper modules are there to reduce repeating code and make code shorter.

What I’m not satisfied with is how the crawl module is done, I may be experiencing the goldilocks effect but I feel like a while task.wait() do loop may be too much for setting the animation speed for when the player is not moving in the crawl module.

I also want my run module and the other pieces of code to be improved through your feedback so I can learn from them and improve my luau programming skills myself.

What potential improvements have you considered?

The potential improvements I’ve considered, on the crawl module is to add a break on the while task.wait() do loop for when it’s currently unused as a way to try to clean things up and hopefully make things more performant.

How (specifically) do you want to improve the code?

As I’ve said before, I want to improve the following code in general from your feedback that I can learn from and improve on.

Specifically though, since the main focus is on the crawl module, I want to make it more performant and I could be experiencing the goldilocks effect but as I’ve said before, I think adding a while task.wait() do loop in the crawl module might impact the performance a little bit. On the other modules, I just want to improve them in general, could be formatting wise, other principles to follow or any other things.

Notes:

Both the crawl module and run module are following Quenty’s principle of Single-Script Architecture so they are part of a “master” LocalScript and why you see functions like Module:Init()
All code are written in Luau and have strict type-checking. Not only that, but all the code shown below will soon be published as a new commit in my UncopylockedPlace GitHub repository. (Edit: Now released, view the commit mentioned here or view the repository here)

Code (Starting from main modules to helper modules):

Crawl module:

--!strict
local Crawl = {}

local ContextActionService = game:GetService("ContextActionService")
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local LocalPlayer = Players.LocalPlayer
local LocalCharacter = LocalPlayer.Character or LocalPlayer.CharacterAdded:Wait()
local Humanoid = LocalCharacter:FindFirstChildOfClass("Humanoid")

local Modules = ReplicatedStorage:FindFirstChild("Modules")
local AnimatorTools = require(Modules:FindFirstChild("AnimatorTools"))
local Mobiler = require(Modules:FindFirstChild("Mobiler"))

local RunModule = script.Parent:FindFirstChild("Run")

local CrawlAnimation = script:FindFirstChild("CrawlAnimation")
local LoadedAnimation: AnimationTrack = AnimatorTools:LoadAnimation(Humanoid, CrawlAnimation) :: AnimationTrack

--local AnimationThread: thread? = nil -- currently unused

local Action = "ACTION_CRAWL"

local Crawling = false

local function HandleAction(ActionName: string, InputState: Enum.UserInputState, _)
	if ActionName == Action then
		if InputState == Enum.UserInputState.Begin then
			Crawling = not Crawling
			
			script:SetAttribute("Crawling", Crawling)

			AnimatorTools:PlayTrack(LoadedAnimation, Crawling)

			Humanoid.WalkSpeed = (Crawling and 4) or (RunModule:GetAttribute("Running") and 24 or 16)

			task.spawn(function()
				while task.wait() do
					if Crawling then
						if Humanoid.MoveDirection.Magnitude == 0 then
							LoadedAnimation:AdjustSpeed(0)
						else
							if RunModule:GetAttribute("Running") then
								Humanoid.WalkSpeed = (Crawling and 6) or (RunModule:GetAttribute("Running") and 24 or 16)
								LoadedAnimation:AdjustSpeed(1.25)
							else
								Humanoid.WalkSpeed = (Crawling and 4) or (RunModule:GetAttribute("Running") and 24 or 16)
								LoadedAnimation:AdjustSpeed(1)
							end
						end
					else
						break -- maybe this is good to do when you want to optimize your code?
					end
				end
			end)

			--dang bruh this shit too long vvv
			--if not Crawling then
			--	LoadedAnimation:Play()

			--	if RunModule:GetAttribute("Running") then
			--		Humanoid.WalkSpeed = 6
			--	else
			--		Humanoid.WalkSpeed = 4
			--	end

			--	Crawling = true
			--else
			--	LoadedAnimation:Stop()

			--	if RunModule:GetAttribute("Running") then
			--		Humanoid.WalkSpeed = 24
			--	else
			--		Humanoid.WalkSpeed = 16
			--	end

			--	task.cancel(AnimationThread)

			--	AnimationThread = nil :: never

			--	Crawling = false
			--end

			-- Crawling speed: 4
			-- Crawling + Sprinting speed: 6

			-- Okay... and how do we check if user's sprinting?
			-- Humanoid.WalkSpeed = (Crawling and 4 (CheckSprinting and 6)) or 16
			-- Answer: communicate using attributes

			-- if Humanoid.MoveDirection.Magnitude == 0 then set anim speed 0 else normal end
		end
	end
end

function Crawl:Init()
	LocalPlayer.CharacterAdded:Connect(function(Character)
		repeat task.wait()
		until Character:FindFirstChildOfClass("Humanoid")

		Humanoid = Character:FindFirstChildOfClass("Humanoid")
		
		LoadedAnimation = AnimatorTools:LoadAnimation(Humanoid, CrawlAnimation) :: AnimationTrack
	end)

	ContextActionService:BindAction(Action, HandleAction, true, Enum.KeyCode.C, Enum.KeyCode.ButtonL3)
	ContextActionService:SetTitle(Action, "Crawl")
	ContextActionService:SetPosition(Action, UDim2.new(0.55, 0, 0.1, 0))

	Mobiler:ModifyTouchButton(Action)
end

return Crawl

Run module:

--!strict
local Run = {}

local ContextActionService = game:GetService("ContextActionService")
local GuiService = game:GetService("GuiService")
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")

local LocalPlayer = Players.LocalPlayer
local LocalCharacter = LocalPlayer.Character or LocalPlayer.CharacterAdded:Wait()
local Humanoid = LocalCharacter:FindFirstChildOfClass("Humanoid")

local Modules = ReplicatedStorage:FindFirstChild("Modules")
local Mobiler = require(Modules:FindFirstChild("Mobiler"))

local CurrentCamera = workspace.CurrentCamera
local OriginalFieldOfView = CurrentCamera.FieldOfView

local CrawlModule = script.Parent:FindFirstChild("Crawl")

local TweenData = TweenInfo.new(0.25)

local Action = "ACTION_RUN"

local RunSpeed = 24

local Running = false

local function HandleAction(ActionName: string, InputState: Enum.UserInputState, Input: InputObject)
	if ActionName == Action then
		if Input.UserInputType ~= Enum.UserInputType.Touch and not string.find(Input.UserInputType.Name, "Gamepad") then
			if InputState == Enum.UserInputState.Begin then
				script:SetAttribute("Running", true)

				TweenService:Create(CurrentCamera, TweenData, {FieldOfView = 75}):Play()

				Humanoid.WalkSpeed = (CrawlModule:GetAttribute("Crawling") and 6) or RunSpeed

				ContextActionService:SetTitle(Action, "Walk")
			elseif InputState == Enum.UserInputState.End then
				script:SetAttribute("Running", false)

				TweenService:Create(CurrentCamera, TweenData, {FieldOfView = OriginalFieldOfView}):Play()

				Humanoid.WalkSpeed = (CrawlModule:GetAttribute("Crawling") and 4) or 16

				ContextActionService:SetTitle(Action, "Run")
			end
		elseif Input.UserInputType == Enum.UserInputType.Touch or string.find(Input.UserInputType.Name, "Gamepad") and InputState == Enum.UserInputState.Begin then
			Running = not Running

			script:SetAttribute("Running", Running)

			TweenService:Create(CurrentCamera, TweenData, {FieldOfView = (Running and 75) or OriginalFieldOfView}):Play()

			Humanoid.WalkSpeed = (CrawlModule:GetAttribute("Crawling") and Running and 6) or (CrawlModule:GetAttribute("Crawling") and 4) or (Running and RunSpeed) or 16

			ContextActionService:SetTitle(Action, (Running and "Walk") or "Run")
		end
	end
end

local function ResetToDefault()
	repeat task.wait()
	until workspace.CurrentCamera

	CurrentCamera = workspace.CurrentCamera

	TweenService:Create(CurrentCamera, TweenData, {FieldOfView = OriginalFieldOfView}):Play()

	Humanoid.WalkSpeed = 16

	ContextActionService:SetTitle(Action, "Run")

	Running = false

	script:SetAttribute("Running", Running)
end

function Run:Init()
	LocalPlayer.CharacterAdded:Connect(function(Character)
		repeat task.wait()
		until Character:FindFirstChildOfClass("Humanoid")

		Humanoid = Character:FindFirstChildOfClass("Humanoid")

		ResetToDefault()
	end)

	GuiService.MenuOpened:Connect(ResetToDefault)

	ContextActionService:BindAction(Action, HandleAction, true, Enum.KeyCode.LeftShift, Enum.KeyCode.RightShift, Enum.KeyCode.ButtonB)
	ContextActionService:SetTitle(Action, "Run")
	ContextActionService:SetPosition(Action, UDim2.new(0.25, 0, 0.3, 0))

	Mobiler:ModifyTouchButton(Action)
end

return Run

Mobiler helper module:

--!strict
local Mobiler = {}

local ContextActionService = game:GetService("ContextActionService")

function Mobiler:ModifyTouchButton(ActionName: string)
	local TouchButton: Instance? = ContextActionService:GetButton(ActionName)

	if TouchButton and TouchButton:IsA("ImageButton") then
		TouchButton.Size = UDim2.new(0.2, 0, 0.2, 0)
		TouchButton.Image = "rbxasset://textures/ui/Input/TouchControlsSheetV2.png"
		TouchButton.ImageColor3 = Color3.new(0, 0, 0)
		TouchButton.ImageRectOffset = Vector2.new(1, 1)
		TouchButton.ImageRectSize = Vector2.new(144, 144)

		local Constraint = Instance.new("UIAspectRatioConstraint")
		Constraint.Name = "Constraint"
		Constraint.AspectType = Enum.AspectType.ScaleWithParentSize
		Constraint.Parent = TouchButton

		local Scaler = Instance.new("UIScale")
		Scaler.Name = "Scaler"
		Scaler.Scale = 1.25
		Scaler.Parent = TouchButton

		TouchButton.Changed:Connect(function()
			TouchButton.Image = "rbxasset://textures/ui/Input/TouchControlsSheetV2.png"
			TouchButton.ImageRectOffset = Vector2.new(1, 1)
			TouchButton.ImageRectSize = Vector2.new(144, 144)
		end)
		
		-- Do these following two functions even serve a proper purpose other than testing?
		TouchButton.MouseButton1Down:Connect(function()
			TouchButton.ImageColor3 = Color3.new(0.75, 0.75, 0.75)
		end)

		TouchButton.MouseButton1Up:Connect(function()
			TouchButton.ImageColor3 = Color3.new(0, 0, 0)
		end)

		TouchButton.TouchLongPress:Connect(function(_, State)
			if State == Enum.UserInputState.Begin then
				TouchButton.ImageColor3 = Color3.new(0.75, 0.75, 0.75)
			elseif State == Enum.UserInputState.End then
				TouchButton.ImageColor3 = Color3.new(0, 0, 0)
			end
		end)

		local ActionIcon = TouchButton:FindFirstChild("ActionIcon")

		if ActionIcon then
			ActionIcon:Destroy()
		end

		local ActionTitle = TouchButton:FindFirstChild("ActionTitle")

		if ActionTitle and ActionTitle:IsA("TextLabel") then
			ActionTitle.FontFace = Font.fromEnum(Enum.Font.BuilderSansBold)
			ActionTitle.TextScaled = true
			ActionTitle.TextTransparency = 0.2

			local TextConstraint = Instance.new("UITextSizeConstraint")
			TextConstraint.Name = "TextConstraint"
			TextConstraint.MaxTextSize = 20
			TextConstraint.MinTextSize = 8
			TextConstraint.Parent = ActionTitle
		end
	end
end

return Mobiler

AnimatorTools helper module:

--!strict
local AnimatorTools = {}

function AnimatorTools:LoadAnimation(Humanoid: Humanoid?, Animation: Animation) : AnimationTrack?
	if Humanoid then
		local Animator = Humanoid:FindFirstChildOfClass("Animator")

		if Animator then
			return Animator:LoadAnimation(Animation)
		end
	end

	return nil
end

function AnimatorTools:PlayTrack(Track: AnimationTrack?, Playing: boolean)
	if Track then
		if Playing then
			Track:Play()
		else
			Track:Stop()
		end
	end
end

return AnimatorTools

SirVicky · April 13, 2024, 6:43pm

while task.wait() do
    if Crawling then
           --
    else
	   break 
    end
end

You should avoid breaks and make use of the loop condition in the while statement: see below code

while Crawling do
    task.wait()
end

repeat task.wait()
until Character:FindFirstChildOfClass("Humanoid")

^ When CharacterAdded is called, the Humanoid is already loaded, so this is unneccessary
You do this in both the Crawl and Run module.

repeat task.wait()
until workspace.CurrentCamera

^ Unless you destroy the camera at some point during your code, this is also unneccessary
If your character is loaded in, the camera will definitely also be there.

while task.wait() do loop may be too much

Alternatively, you can use :GetPropertyChangedSignal for the MoveDirection of the Humanoid, so it will just fire specifically when the MoveDirection changes rather than running the loop every heartbeat.
Documentation

Other than that, the code itself is clean! Or at least cleaner than mine haha

debugMage · April 14, 2024, 9:56pm

Do you know this to be a fact? I wish there were documentation explaining what is always loaded and when it is.

SirVicky · April 16, 2024, 9:41pm

There is no way to prove it, ultimately, but the camera is always one of the first to be loaded because it is important. If you try to destroy the current camera, the game instantly replaces it; so it must be significant. Along with other core roblox scripts that are hidden from us, it will always be one of the first to load.

system · April 30, 2024, 9:42pm

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.