DRY principle:
You can make your life and your code easier by thinking of the Don’t Repeat Yourself (DRY) principle. It aims to avoid typing the same thing over and over again when it’s more useful to use some kind of abstraction. Instead of specifying exactly how each item has to be set up, we abstract all of that into a mental model of an “item”. An “item” is a GuiObject that has a BuyFrame, a ClickSound, an associated ViewFrame and some other things. When an “item” is clicked, it’s ViewFrame is showed and all others are hidden.
That code is repeated a lot, so that’s an obvious first place to DRY up your code. E.g.:
function hideAllFrames()
ItemsFrame.Visible = false
CrowbarViewFrame.Visible = false
MedKitViewFrame.Visible = false
KeyDetectorViewFrame.Visible = false
BackpackViewFrame.Visible = false
end
function showFrame( frame )
hideAllFrames()
frame.Visible = true
end
Now you can simplify each MouseButton1Click listener function a lot:
Backpack.BuyFrame.Button.MouseButton1Click:Connect(function()
Backpack.BuyFrame.Button.ClickSound:Play()
showFrame(BackpackViewFrame)
end)
Torch.BuyFrame.Button.MouseButton1Click:Connect(function()
Torch.BuyFrame.Button.ClickSound:Play()
showFrame(TorchViewFrame)
end)
Now, most of the repetition is in setting up the listener functions. To DRY it up even more, we can loop over all the items and set them up in the same manner:
local items = ItemsFrame:GetChildren() --This might need to filter out other GuiObjects
for _, item in pairs(items) do
--You might want an assert here to see if the Button exists.
item.BuyFrame.Button.MouseButton1Click:Connect(function()
item.BuyFrame.Button.ClickSound:Play()
local itemViewFrame = MainFrame:FindFirstChild(item.Name .. "ViewFrame")
showFrame( itemViewFrame )
end)
end
Not only do you avoid having to type out a bunch of code for each item, if you want to add another item later it’s as simple as setting up the GUI (button, view frames) and it should magically work on it own.
We can actually “simplify” and generalize the hideAllFrames
function in the same way:
function hideAllFrames()
for _, item in pairs(items) do
local itemViewFrame = MainFrame:FindFirstChild(item.Name .. "ViewFrame")
itemViewFrame.Visible = false
end
end
You can also DRY up the last two connections in the same manner.
Here's how I would DRY up your entire script
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Player = Players.LocalPlayer
local MainFrame = script.Parent:WaitForChild("Frame")
local ItemsFrame = MainFrame:WaitForChild("ItemsFrame"):WaitForChild("ScrollingFrame")
local RemoteFunctions = ReplicatedStorage.RemoteFunctions
local RemoteEvents = ReplicatedStorage.RemoteFunctions
function hideAllFrames()
for _, item in pairs(items) do
local itemViewFrame = MainFrame:FindFirstChild(item.Name .. "ViewFrame")
itemViewFrame.Visible = false
end
end
function showFrame( frame )
hideAllFrames()
frame.Visible = true
end
function setupItem( item )
--Setup item buttons (click sounds, hiding and showing frames)
item.BuyFrame.Button.MouseButton1Click:Connect(function()
item.BuyFrame.Button.ClickSound:Play()
local itemViewFrame = MainFrame:FindFirstChild(item.Name .. "ViewFrame")
if itemViewFrame then
showFrame( itemViewFrame )
ItemsFrame.Visible = false
end
end)
end
function setupViewFrame( viewFrame )
--Setup some other stuff, assuming all the viewFrames work the same way
local n = viewFrame.Name
local itemName = string.sub(n, 1, string.find(n, "ViewFrame") - 1)
local buyFrame = viewFrame[itemName].BuyFrame
buyFrame.Button.MouseButton1Click:Connect(function()
buyFrame.Button.ClickSound:Play()
RemoteFunctions[itemName]:InvokeServer(buyFrame.Cost.Value, buyFrame.Level.Value)
end)
end
function setup()
items = ItemsFrame:GetChildren() --This might need to filter out other GuiObjects
viewFrames = MainFrame:GetChildren() --Same here
for _, item in pairs(items) do
setupItem(item)
end
for _, viewFrame in pairs(viewFrames) do
setupViewFrame(viewFrame)
end
end
--Remember to call the function :)
setup()
return true