Can my code cause memory leaks?

Help and Feedback Code Review
1

Can this code cause memory leaks?

local Wheat = game:GetService("Workspace"):WaitForChild("Wheat")
local Sugar = game:GetService("Workspace"):WaitForChild("Sugar")

local Donut = game:GetService("Workspace"):WaitForChild("Donut")

local clicks = {Wheat.Click, Sugar.Click, Donut.ProximityPrompt}

local Connections = {}

local questDone = {}

local questStarted = game:GetService("ReplicatedStorage"):WaitForChild("QuestStarted")

local function sugarFunction(plr)
    if plr:FindFirstChild("QuestStarted") and plr:FindFirstChild("Type") and plr.Type.Value == "Donut" then

        if not game:GetService("ReplicatedStorage").QuestStarted.Value then return end

        if not plr:FindFirstChild("Sugar") then
            local sugar = Instance.new("StringValue")
            sugar.Name = "Sugar"
            sugar.Parent = plr

            local objective = game:GetService("ReplicatedStorage").Remotes.Objective    

            local progress = 0
            if plr:FindFirstChild("Sugar") and plr:FindFirstChild("Wheat") then
                progress = 2;
            elseif plr:FindFirstChild("Wheat") then
                progress = 1;
            elseif plr:FindFirstChild("Sugar") then
                progress = 1
            end

            if progress == 2 then
                objective:FireClient(plr, "Return them to the donut shop!", true, true, progress, 2)
            else
                objective:FireClient(plr, nil, true, true, progress, 2)
            end
            
            objective = nil
            sugar = nil
        end
    end
end

local function wheatFunction(plr)
    if plr:FindFirstChild("QuestStarted") and plr:FindFirstChild("Type") and plr.Type.Value == "Donut" then

        if not game:GetService("ReplicatedStorage").QuestStarted.Value then return end

        if not plr:FindFirstChild("Wheat") then
            local wheat = Instance.new("StringValue")
            wheat.Name = "Wheat"
            wheat.Parent = plr

            local objective = game:GetService("ReplicatedStorage").Remotes.Objective            

            local progress = 0

            if plr:FindFirstChild("Sugar") and wheat then
                progress = 2;
            elseif plr:FindFirstChild("Wheat") then
                progress = 1;
            end

            if progress == 2 then
                objective:FireClient(plr, "Return them to the donut shop!", true, true, progress, 2)
            else
                objective:FireClient(plr, nil, true, true, progress, 2)
            end
            
            objective = nil
            wheat = nil
            
        end
    end
end

local function onDonut_Trigerred(plr)
    local dialog = game:GetService("ReplicatedStorage").Remotes.Dialog;
    local objective = game:GetService("ReplicatedStorage").Remotes.Objective

    if not game:GetService("ReplicatedStorage").QuestStarted.Value then return end

    if table.find(questDone, plr) then return end;

    if not plr:FindFirstChild("QuestStarted") then

        local started = Instance.new("BoolValue")

        started.Name = "QuestStarted";

        started.Parent = plr

        dialog:FireClient(plr, "<AnimateStepFrequency=1>Hello,<AnimateYield=1> looks like I'm running out of donuts.<AnimateYield=1> Mind going to the nearest convenience store to get me some flour and Sugar?", 0.065, "rbxassetid://6173636603", "Donut Lady")

        wait(12)

        if not game:GetService("ReplicatedStorage").QuestStarted.Value then return end

        local questType = Instance.new("StringValue")

        questType.Name = "Type"

        questType.Value = "Donut"

        questType.Parent = plr

        objective:FireClient(plr, "Collect flour and sugar from the store!", true, true, "0", "2")

        dialog:FireClient(plr, true)
        
        dialog = nil
        
        objective = nil
        
        questType = nil
        
        started = nil

    elseif plr:FindFirstChild("Wheat") and plr:FindFirstChild("Sugar") then

        table.insert(questDone, plr)

        plr.Wheat:Destroy();

        plr.Sugar:Destroy()

        plr.QuestStarted:Destroy()

        plr.Type:Destroy()

        dialog:FireClient(plr, "<AnimateStepFrequency=1>Thank you for your <AnimateStyle=Wiggle>generosity<AnimateStyle=/>,<AnimateYield=1> here is a donut you've earned.<AnimateYield=1> *PSST*<AnimateYield=1> It's a special donut", 0.055)

        local Completed = Instance.new("StringValue")

        Completed.Name = "Donut";

        Completed.Parent = plr

        local NewDonut = Donut:Clone()

        NewDonut.Parent = plr.Backpack

        if plr:FindFirstChild("Fishing") and plr:FindFirstChild("Donut") then

            objective:FireClient(plr, nil, false, false)

        elseif plr:FindFirstChild("Donut") then

            objective:FireClient(plr, "Explore the map and do quests!", true, false);
        end

        wait(8)

        dialog:FireClient(plr, true)
        
        Completed = nil
        
        dialog = nil
        
        NewDonut = nil
    end
end

for index, click in pairs(clicks) do
    if click.Parent == Wheat then
        Connections[#Connections + 1] = click.MouseClick:Connect(wheatFunction)
    elseif click.Parent == Sugar then
        Connections[#Connections + 1] = click.MouseClick:Connect(sugarFunction)
    elseif click.Parent == Donut then
        Connections[#Connections + 1] = click.Triggered:Connect(onDonut_Trigerred)
    end
end

Connections[#Connections + 1] = questStarted:GetPropertyChangedSignal("Value"):Connect(function()
    if not (questStarted.Value) then
        for index, connection in pairs(Connections) do
            connection:Disconnect()
        end
        clicks = nil
        
        Wheat = nil
        
        Sugar = nil
        
        Connections = nil
        
        questStarted = nil
        
        Donut = nil
    end
end)
1 Like
2

I didn’t really read all of your code but instead of asking if it can, you should ask how do I know if it can cause memory leaks. Any arrays/tables you don’t set to nil after you are done with them will keep getting bigger and start causing problems. Just make sure you are using the correct scope of variables and you will be fine.

2 Likes
3

Whatever you read, were there any problems that could possibly leak?

4

Not that I saw. To me it looked like you managed your variables properly by emptying them when not needed.

5

I set all my variables to nil after I’m done with them

6

As long as you’re properly storing your connections and then disconnecting them, you should be fine.

It’s up to you, but you might benefit a lot from making a custom connection handler (a.k.a, a maid) or using one that’s already been scripted.

1 Like
7

It shouldn’t cause a memory leak. Also setting variables allocated on the stack to nil is redundant since they are cleared from the memory when the stack ends assuming they have no strong references.

1 Like