Best way to format this code loop?

I have this section of code in my script, and it works perfectly fine, performance and function-wise. I just don’t like how it looks, is there another way to write this loop better while still performing the same function / loop?

local animateConnection

textButton.MouseButton1Click:Connect(function()
     local function animate()
		local Tween = Utilities.tween(BackgroundContainer.ImageLabel, {["tweenInfo"] = onClickInfo, ["easingStyle"] = Enum.EasingStyle.Linear, ["duration"] = 6}, {Position = UDim2.new(0, 0, -1)})
		animateConnection = Tween
				
		Tween.Completed:Connect(function()
			animate()
		end)
	end

	if animateConnection then
		animateConnection:Play()
	else
		animate()
	end
end)

exitButton.MouseButton1Click:Connect(function()
	animateConnection:Pause()
end)

First off you should use Tween.Completed:Wait() so you don’t actually need a function. Also in pretty sure there will be multiple connect functions which isn’t good.

You’re also creating another variable “Tween” which you don’t need to do. Just set it directly to the other variable.

Also why do you call animate again? It doesn’t play the animation it just sets it. If you’re trying to create a loop don’t you have to play the animation? Could be my mistake tho. Edit: I just read it and I realized I’m wrong. Still kind of confused tho lol.

Also you don’t really have to name each enum for the tween. It could be your preference tho.

MouseButton1Down is also better just in case you want mobile support.

Lastly, this is kind of my opinion, but you shouldn’t put the animate function inside the click function. Looks nicer imo.

I’m probably wrong on some things due to little info. But, anyways good luck.

1 Like

In my opinion, wow, it looks great already as it is, it’s very nice! But how about formatting it like this if it is really needed?

It feels easier to read if some of those lines are commented and written in one line.

local animateConnection

textButton.MouseButton1Click:Connect(function()
     local function animate()
		local Tween = Utilities.tween(BackgroundContainer.ImageLabel, {["tweenInfo"] = onClickInfo, ["easingStyle"] = Enum.EasingStyle.Linear, ["duration"] = 6}, {Position = UDim2.new(0, 0, -1)})
		animateConnection = Tween
				
		Tween.Completed:Connect(animate) -- Restart the tween when it completes
	end

	if animateConnection then animateConnection:Play() else animate() end -- -- Start the tween if it's not already playing
end)

exitButton.MouseButton1Click:Connect(function()
	animateConnection:Pause() -- Pause the tween when the exit button is clicked
end)

I dont think it is nice right now.
Currently you are creating a tween each time player clicks the button and add a connection to it, i think this makes it memory leak. You do not destroy the tween at any time aswell.
I would only create one tween and use that.

local Tween: Tween?

textButton.MouseButton1Click:Connect(function()
    if not Tween then --save some memory incase player never clicks
        Tween = Utilities.tween(BackgroundContainer.ImageLabel, {["tweenInfo"] = onClickInfo, ["easingStyle"] = Enum.EasingStyle.Linear, ["duration"] = 6}, {Position = UDim2.fromScale(0, -1)})
        Tween.Completed:Connect(function()
            Tween:Play()
        end)
    end

    if Tween.PlaybackState ~= Enum.PlaybackState.Playing then
        Tween:Play()
    end
end)

exitButton.MouseButton1Click:Connect(function()
    if not Tween then
        return
    end
	Tween:Pause()
    --[[
    --You can destroy tween after some time, if you want. Like if player never clicked it again.
    --That should delete the connections and save very little memory, only use if your stuff gets complex!
    local currentStopTask --at top

    --in exitButton
    if currentStopTask then
        currentStopTask:Cancel()
        currentStopTask = nil
    end

    currentStopTask = task.delay(5, function()
        Tween:Destroy()
        Tween = nil
    end)
    ]]
end)

oh mb i didn’t see the

what you are doing is nice, just need to modify exitButton to check if animateConnection exists

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