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.
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)