Why is this script so laggy?

  1. What do you want to achieve?

Not laggy script.

  1. What is the issue?

The issue is it laggs when ever you spam click!

  1. What solutions have you tried so far?

Finding a solution!

local plr = game.Players.LocalPlayer
local lockimage = "http://www.roblox.com/asset/?id=2936897307"
local name = script.Parent.name.Value
local locker = plr.Locker

while wait() do
    
    if locker[script.Parent.Name].Value == false then

        script.Parent.ImageLabel.Image = lockimage
        script.Parent.ImageLabel.ImageColor3 = Color3.new(255, 255, 255)
        script.Parent.ImageColor3 = Color3.new(0.486275, 0.486275, 0.486275)
        script.Parent.Icon.Visible = false
    else
        if locker[script.Parent.Name].Value == true then
            script.Parent.Icon.Visible = true
            script.Parent.ImageColor3 = Color3.new(255,255,255)
            script.Parent.ImageLabel.Visible = false

            script.Parent.MouseButton1Click:Connect(function()
                script.Parent.Parent.Parent.SelectedPack.Value = name
                script.Parent.Parent.Parent.Storage.Value = script.Parent.Storage.Value
                script.Parent.Parent.Parent.Cost.Value = script.Parent.Cost.Value    
                script.Parent.Parent.Parent.ToUnlock.Value = script.Parent.ToUnlock.Value
                script.Parent.Parent.Parent.SelectedImage.Value = script.Parent.Icon.Image




            end)
        end
    end
end

Thank you in advance!

From the looks of it, it changes a bunch of elements, no matter what. It doesn’t really seem to have a debounce feature. Maybe add a few wait statements in between changing this a bunch.

Where would I add the debounce though?

You can pick! I would do something like this though:

local plr = game.Players.LocalPlayer
local lockimage = "http://www.roblox.com/asset/?id=2936897307"
local name = script.Parent.name.Value
local locker = plr.Locker

while wait() do
    
    if locker[script.Parent.Name].Value == false then

        script.Parent.ImageLabel.Image = lockimage
        script.Parent.ImageLabel.ImageColor3 = Color3.new(255, 255, 255)
wait(2)
        script.Parent.ImageColor3 = Color3.new(0.486275, 0.486275, 0.486275)
        script.Parent.Icon.Visible = false
    else
        if locker[script.Parent.Name].Value == true then
            script.Parent.Icon.Visible = true
wait(2)
            script.Parent.ImageColor3 = Color3.new(255,255,255)
            script.Parent.ImageLabel.Visible = false

            script.Parent.MouseButton1Click:Connect(function()
                script.Parent.Parent.Parent.SelectedPack.Value = name
wait(2)
                script.Parent.Parent.Parent.Storage.Value = script.Parent.Storage.Value
                script.Parent.Parent.Parent.Cost.Value = script.Parent.Cost.Value    
wait(2)
                script.Parent.Parent.Parent.ToUnlock.Value = script.Parent.ToUnlock.Value
                script.Parent.Parent.Parent.SelectedImage.Value = script.Parent.Icon.Image




            end)
        end
    end
end

But now whenever I click it takes too long

First of all, wait() is going to get deprecated soon, use task.wait, and dont use while wait do, use

While true do

task.wait()
end

, If you want it faster I recommend you do this instead

locker[script.Parent.Name]:GetPropertyChangedSignal(“Value”):Connect(function()

3 Likes

It yields the thread attempting to look for its said params. Find some other event to use to substitute your while loops instead of wait(). Like @regexman said you can do while (true). Personally I dont think thats the greatest option as you can have RunService or (in this case I dont know exactly what your code does) a :Changed() event.

2 Likes

I suspect that your biggest issue is that this line of code:

is connecting a new anonymous click-handling function every time the loop loops. Then, when you do click, all of them are called (hundreds, possibly thousands of functions, depending on how many times the while loop has run due to locker[script.Parent.Name].Value == true).

The most obvious thing to do is just assign the connection handler to a variable that is declared outside the while loop, e.g.
myClickHandler = script.Parent.MouseButton1Click:Connect(function() <your code> end)
Keeping a variable reference to the connection means you can call myClickHandler:Disconnect() when you no longer need to be listening for mouse clicks, and set myClickHandler = nil so that you don’t make duplicates if it’s already connected:

if not myClickHandler then
    myClickHandler = script.Parent.MouseButton1Click:Connect(function() <your code> end)
end

Really though, the proper solution is to not have things inside a loop that aren’t meant to run repeatedly. If what you really care about is executing code when locker[script.Parent.Name].Value changes value, then you should set up a connection to listen for changes to that value object, rather than polling with a while(true) loop. This is what @regexman suggested above, but I’d add to that suggestion that I think it’s generally the best practice to use the return value from all Connect() calls. Connect() returns an RBXScriptConnection, and if you don’t save it to a variable, you can’t call Disconnect()! Even in cases where it might seem OK for the connection to have the same lifetime as the Instance generating the events, you might still want to explictly control the disconnect. You can get some hard-to-find bugs if you just leave it to garbage collection to clean up all your connections.

2 Likes

Thank you so much for everyone who is trying to help, when I get on pc I’ll try your solution’s!

Try this (this is with everyone’s compiled answers with my answers)

local plr = game.Players.LocalPlayer
local lockimage = "http://www.roblox.com/asset/?id=2936897307"
local name = script.Parent.name.Value
local locker = plr.Locker

script.Parent.MouseButton1Click:Connect(function()
    if locker[script.Parent.Name].Value == true then
        script.Parent.Parent.Parent.SelectedPack.Value = name
        script.Parent.Parent.Parent.Storage.Value = script.Parent.Storage.Value
        script.Parent.Parent.Parent.Cost.Value = script.Parent.Cost.Value    
        script.Parent.Parent.Parent.ToUnlock.Value = script.Parent.ToUnlock.Value
        script.Parent.Parent.Parent.SelectedImage.Value = script.Parent.Icon.Image
    end
end)

local function changedVal()
    if locker[script.Parent.Name].Value == false then
        script.Parent.ImageLabel.Image = lockimage
        script.Parent.ImageLabel.ImageColor3 = Color3.new(255, 255, 255)
        script.Parent.ImageColor3 = Color3.new(0.486275, 0.486275, 0.486275)
        script.Parent.Icon.Visible = false
    elseif locker[script.Parent.Name].Value == true then
        script.Parent.Icon.Visible = true
        script.Parent.ImageColor3 = Color3.new(255,255,255)
        script.Parent.ImageLabel.Visible = false
    end
end

locker[script.Parent.Name]:GetPropertyChangedSignal("Value"):Connect(function()
    changedVal()
end)

changedVal()
1 Like

Thank you so much it works! Thank you everyone who helped!