Gui performance

[Blue249_02 (BlueFox)]

This code runs when a text button is clicked on and copies a model into a viewport frame (Basically a shop gui that I display the models in; I’ve coded this part). When the player closes the detailed view of that and opens a description of another model the code deletes the model inside the viewport frame and add the model of a new one (I’ll code this later).

As I’m a new developer I’m not really sure if this is performance effective or not.

P.S: Tower defense game.

--Variables
local Gui = game.Players.LocalPlayer.PlayerGui:WaitForChild("Shop")
local MainFrame = Gui:WaitForChild("MainFrame")
local BaseFrame = MainFrame:WaitForChild("BaseFrame")
local Scrolling = BaseFrame:WaitForChild("ScrollingFrame")
local PlasmoButton = Scrolling:WaitForChild("PlasmoButton")
local InfoFrame = MainFrame:WaitForChild("InfoScreen")
local Frame = InfoFrame:WaitForChild("Frame")
local Oilivia = game.Workspace:WaitForChild("Towers"):WaitForChild("Oilivia")
local Plasmo = game.Workspace:WaitForChild("ShopStuff (Don't delete)")

-----------------------------------------------------------------------------------------------------------------

--Change Screens based on buttons and add Tower model's
PlasmoButton.MouseButton1Up:Connect(function()
	BaseFrame.Visible = false
	InfoFrame.Visible = true
	for _,v in pairs(Plasmo:GetChildren()) do 
		local clone = v:Clone()
		clone.Parent = game.Workspace
		clone:WaitForChild("HumanoidRootPart").CFrame = CFrame.new(102.82, -254.96, 269.366)
		if v.Name == "PlasmoLevel1" then 
			clone.Parent = Frame:WaitForChild("Level 1")
		elseif v.Name == "PlasmoLevel2" then 
			clone.Parent = Frame:WaitForChild("Level 2")
		elseif v.Name == "PlasmoLevel3" then
			clone.Parent = Frame:WaitForChild("Level 3")
		end
	end
	local clone = Plasmo.PlasmoLevel1:Clone()
	clone.Parent = game.Workspace
	clone:WaitForChild("HumanoidRootPart").CFrame = CFrame.new(102.82, -254.96, 269.366)
	clone.Parent = InfoFrame:WaitForChild("ViewportFrame")
end)

[soksakCZ (soki)]

to me the code looks fine, but I’m not that much of a scripter. I still have some general tips

• Instead of MouseButton1Up use the Activated event. It works better for the user. It also works on all devices, not only with mouse.
• Also when using a for _ in *table* do loop you don’t have to put your table inside pairs(). In fact, it worse to do so.
• And maybe some of the WaitForChilds aren’t necessary. I don’t think they’ll negatively impact the performance but it might be too much to look at. If you want it to be “error free” you could use FindFirstChild, which returns nil if the Instance isn’t found.
• I’m pretty sure you have a little too much WaitForChilds everywhere, but I don’t know where exactly they’re necessary. You might wanna look at some replication documentation or something where this should be explained. I remember looking at it but idk.
• In general this seems like it has fine performance. Only thing you could improve (which’ll be REALLY unnecessary and not save that much performance) is disconnect events from gui objects when you know you won’t need them. Like when a gui isn’t visible just disable the all the hover, click and input events.
• Roblox has some tutorials for performance that you could check out.

Hope this helps feel free to ask anything

[scavengingbot]

@soksakCZ

Also when using a for _ in *table* do loop you don’t have to put your table inside pairs(). In fact, it worse to do so.

It is better depending on which you chose.
if you say: for i, v in table do then roblox automatically chooses between ipairs and pairs.
so if you have:

local dictionary = { ["hey"] = 5, ["hi"] = 6 ... }
for _, v in pairs(dictionary) do
...
end

then it is faster, because roblox doesn’t have to choose anything anymore, since it is already correct.

but using it with the wrong kind of table makes it slower:

local dictionary = { ["hey"] = 5, ["hi"] = 6 ... }
for _, v in ipairs(dictionary) do --ipairs is for arrays, not key value tables
...
end

if you use the wrong iterator function then it is better to use roblox’ automatic choosing.
but it is faster if you just use the correct iterator.

pairs for dictionaries
ipairs for arrays.

in this case,:

@Blue249_02

for _,v in pairs(Plasmo:GetChildren()) do

it is worse than roblox’ automatic choosing because it is the wrong iterator.
But prefixing it with an I and boom it is suddenly slightly faster.
But the difference is minimal and most games don’t need optimization like this…

[Blue249_02 (BlueFox)]

Thanks, it did help me and now I think my code will be better performance wise.