Is there a problem in this script?

Help and Feedback Code Review
#1

Hello! I am Uniqueoq and I was scripting where the script is supposed to play an animation whenever the character uses the object. Here is the script. Anybody please tell me what the mistake is! Thank you!

local object = script.Parent

local equipped = false

local plr = game.Players.LocalPlayer

local mouse = plr:GetMouse()

local liftingId = “rbxassetid://3996845349”

local animation = Instance.new(“Animation”)
animation.AnimationId = liftingId

object.Equipped:Connect(function()
equipped = true
end)

object.Uneqquiped:Connect(function()
equipped = false
end)

mouse.Button1Down:Connect(function()
if equipped then
local liftAnim = plr.Character.Humanoid:LoadAnimation(animation)

	liftAnim:Play()
end

end)

1 Like
#2

Your code is a little redundant, it can be cleaned up pretty easily.

The .Equipped event returns a mouse instance. Most of your code can be broken down into:

object.Equipped:Connect(function(mouse)
    mouse.Button1Down:Connect(function()
        -- Logic.
    end)
end)

…there isn’t a need to listen for the tool .Unequipped event (unless you would like to terminate the playing animation once the tool is unequipped) since once the tool is unequipped the connection listening to Button1Down should terminate.

It might also be worth checking whether the animation is already playing to prevent it from reloading every click. You can utilise the IsPlaying property of the AnimationTrack to do this.

Ultimately, it should look along the lines of:

local object = script.Parent
local plr = game:GetService("Players").LocalPlayer
local character = plr.Character or plr.CharacterAdded:Wait()
local humanoid = character:WaitForChild("Humanoid")

local liftingId = "rbxassetid://3996845349"
local animation = Instance.new("Animation")
animation.AnimationId = liftingId
local liftAnim = humanoid:LoadAnimation(animation)

object.Equipped:Connect(function(mouse)
    mouse.Button1Down:Connect(function()
        if not liftAnim.IsPlaying then
            liftAnim:Play()
        end
    end)
end)
1 Like
#3

You’re setting a variable as that animation id, but you’re not doing anything with that variable.

You will want to load that animation into the player’s humanoid and play and stop whenever required.

local AnimationTrack = plr.Character.Humanoid:LoadAnimation(animation)
AnimationTrack:Play()

Edit: Seems like you were, just that I missed it due to the code not being formatted. You need to load it much earlier on rather than attempting to do so when the player clicks. What @byc14 has provided should be correct, as you only need to play the animation when needed. If the animation is a loopable one, you’ll need to :Stop() it afterwards. Otherwise, just :Play() is fine. The animation can be loaded at all times for this.

#4

Alright! Thank you for the great tip! It will help me out a lot! Thanks!

1 Like
#5

Thank you for the response! (mustbe30char)