Load animation script

Help and Feedback Code Review
#1

so this code is loading animation to a model.
i feel like that using while loop here isn’t really efficient, i know there’s an humanoid event for this “changedState”, but it won’t work, so is that okay if i use the while loop here? or is there a way to make it more efficient?

local animtrack = nil
local Humanoid = script.Parent:WaitForChild("Humanoid")
local animator = Humanoid.Animator
local moved = false

local function playAnimation(animation)
    animtrack = animator:LoadAnimation(script[animation])
    animtrack:Play()
end

local function stop()
    if animtrack then
        animtrack:Stop()    
    end
end

while wait() do
    if Humanoid.MoveDirection.Magnitude == 0 and moved then
        -- not moving
        stop()
        moved = false
        playAnimation("idle")
    elseif Humanoid.MoveDirection.Magnitude ~= 0 and not moved then
        -- moving
        stop()
        moved = true
        playAnimation("walk")
    end
end
2 Likes
#2

I think you need an animation object in the script. Otherwise i’m not sure.

Sorry and Good Luck.

#3

i assume you are trying to see if the player move well there is an event for that its

Humanoid.Running:Connect(function(speed)  -----absolute speed of the player
if speed == 0 and moved then
        -- not moving
        stop()
        moved = false
        playAnimation("idle")
    elseif speed ~= 0 and not moved then
        -- moving
        stop()
        moved = true
        playAnimation("walk")
    end
end)
#4

Hi!

@MrchipsMa already gave you the answer (humanoid.Running event), so I’m only adding some other small corrections to your code.

Instad of creating “moved” variable, you can simple check whether animation is already playing or not. You should also load animations only once, as further loading is unnecessary.

At the same time, the code is short, so you don’t have to create an aditional functions to play and stop animations. Furthermore, it’s slightly more performance friendly to not call functions rapidly. This case is not an example of that being a bad practice, but is certainly useful to know that.

This script belongs in StarterCharacterScripts.

local player = game:GetService("Players").LocalPlayer
if (not player.Character) then player.CharacterAdded:Wait() end
local humanoid = player.Character:WaitForChild("Humanoid")
local animator = humanoid:WaitForChild("Animator")

-- Load animation tracks only once!
local idle_track = animator:LoadAnimation(script["idle"])
local walk_track = animator:LoadAnimation(script["walk"])

humanoid.Running:Connect(function(speed)
	-- Check whether animation is playing already.
	if (speed ~= 0 and not walk_track.IsPlaying) then
		idle_track:Stop(); walk_track:Play()
	elseif (speed == 0 and not idle_track.IsPlaying) then
		walk_track:Stop(); idle_track:Play()
	end
end)

Read the following just in case you are curious.

Summary

Previously, you were using MoveDirection. That is a unit vector. It’s magnitude can be either 0 or 1, with one meaning player is moving.

How about magnitude itself? It is a square rooted dot product. Square roots are not so expensive, but if they can be avoided, they should be.

The following two statements calculate exactly the same result:

humanoid.MoveDirection.Magnitude =
= math.sqrt(humanoid.MoveDirection:Dot(humanoid.MoveDirection))

-- Since MoveDirection's magnitude can be either 1 or 0:
Vector3.new(1,0,0):Dot(Vector3.new(1,0,0)) = 1   ;   math.sqrt(1) = 1
Vector3.new(0,0,0):Dot(Vector3.new(0,0,0) = 0    ;   math.sqrt(0) = 0

We can thus skip square root operation!

if (humanoid.MoveDirection:Dot(humanoid.MoveDirection) ~= 0) then ... end
3 Likes
#5

thank you so much!!! I learned a lot from your reply.
btw i’m not trying to load the animation to a player, but to a npc.

1 Like