Custom movement animation "lag"

My goal is for my animation to sync with movement more precisely, because at the moment it has this awful laggy feeling.

I want to know if there is a better way of doing it that feels smoother.

Here is the code I use whenever I want to modify movement:

task.spawn(function()
    local movement
    movement = char:WaitForChild("Humanoid").Running:Connect(function(speed)
        -- Cleanup
        if StateManager.GetState(char, "movingWithDolly") ~= true then track3:Stop() animation3:Destroy() movement:Disconnect() end

        if speed == 0 then
	    track3:Stop()
	else
		if not track3.IsPlaying then
			track3:Play()
		end
	end
end)

Additional information:

  • The statemanager “movingWithDolly” flag is set to true whenever you equip a dolly and set to false/nil when unequipping. Animation is always handled after this.
  • Track3 is the animation track of the animation that is obtained from my animation modulescript function. Animation3 is also from the same function.

Thank you for reading, I will attach a video for a visual of what I’m talking about.

Video: https://drive.google.com/file/d/1jTJlw1pY1GtSY_KpcyXWgx7arv2couaW/view?usp=sharing

(Btw a “dolly” is that thing you user to wheel heavy things around like furniture)

1 Like

For starters, the code you are using is not a good way to check player movement and itself is super sloppy. Some advice, there is zero reason to use the task.spawn coroutine and I would never write all that clean up in a single line. Im talking about the

if StateManager.GetState(char, "movingWithDolly") ~= true then track3:Stop() animation3:Destroy() movement:Disconnect() end

This is a pain to read, and does nothing other than make it harder to read lol. When I was first programming I used to think less lines means better code. This is not true, while it is best to solve the problem with the easiest form of logic your goal should not be to maximize the least amount of lines. Never sacrifice readability. Also, unless you are purposely creating a new animation 3 every time you could simply just reuse the animation 3 instance and not create a new one each time. Meaning don’t destroy it just stop the track.

A better way to write this line would be

local isMovingWithDolly = StateManager.GetState(char, "movingWithDolly")

if not isMovingWithDolly then
     track3:Stop()
     animation3:Destroy()
     movement:Disconnect()
end

Alright that was just some advice. As I was saying before, using .Running is not the best way to figure out if the player is currently moving or not. The Humanoid has a property called MoveDirection which is a Vector3 value. This updates as soon as the player starts moving. We can listen to this change and connect a function to run whenever the change occurs. Heres a little example:

--//Services
local players = game:GetService("Players")

--//Variables
local player = players.LocalPlayer
local character = player.Character
local humanoid = character.Humanoid

--//Functions
local function onPlayerMoved() 
   local currentMoveDirection = humanoid.MoveDirection
   local usingDolly = StateManager.GetState(character, "movingWithDolly")
   
   --> stopping code if we are not moving or if we are not using the dolly
   if currentMoveDirection == Vector3.new(0, 0, 0) or not usingDolly then
       if track3.IsPlaying() then 
           track3:Stop()
        end
      
       return
   end
 
   --> playing animation now that we know we are moving and using the dolly
   if not track3.IsPlaying() then
      track3:Play()
   end
end

--//Signals
Humanoid:GetPropertyChangedSignal("MoveDirection"):Connect(onPlayerMoved)

All I did here was improve the way you were checking if the player was moving or not and cleaned up the overall function along with removing the useless task.spawn call. Task.spawn is useful when you want code to run parallel to each other and if one yields it does not affect the rest of the code base. In the function you displayed, there was no yielding code meaning causing this to run as a thread was completely pointless. I typed this all out without doing any tests and i do not know the overarching format of your code base so do not just plug this in and expect it to work. Instead, read and learn from this and implement it in yourself. Let me know if you have any questions! Happy Programming :smile:

2 Likes

Thank you very much for your advice and help! Had no idea about the MoveDirection property prior.

The task.spawn was used because in my whole code I had stuff running afterwards, but I should not have included it in my example for readability reasons.

Sorry about my code being hard to read, since I am a solo dev learning out I figured as long as I understood it myself there was no reason to change it… probably should have fixed it up before asking about it haha.

I edited it a little and tweaked it to work with my codebase and it was nice and smooth!

1 Like

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