In the last 10 projects I’ve made, each one progressively getting more efficient and easier to develop than burning out because of how messy / bad everything is; I’ve almost always encountered an issue that I’ve been told not to care about and it didn’t look like it was breaking anything, but it really bothers me and I will provide some examples to explain what I’m trying to say better. (I’m not asking for improving the code here I’m asking a general question w examples)
Especially whenever I need a GUI of some sort with buttons on it related to “selecting” things, for every single instance of a button or clickable object that needs to do something if it is clicked, the code often goes something like this:
for thing, _ in pairs(Data) do
local NewButton = Instance.new("ImageButton", GUI)
-- ... configure the button design and all that
NewButton.MouseButton1Click:Connect(function()
-- Do stuff
end
end
In the end, there is like tens maybe hundreds of buttons that all just have this connection.
And also whenever I have something like an Item or Cost that is displayed somewhere, there is always a .Changed or :GetPropertyChangedSignal assigned to every instance like this:
-- ... code that triggered getting an item or storing the item
local NewItem = ItemReference:Clone()
-- ... some sort of textlabel or billboardgui or tool description to tell you the amount
local IV = Instance.new("IntValue", NewItem) -- or SetAttribute
IV.Name = "Amount" -- Convenience
IV.Value = InitialAmount
IV.Changed:Connect(function(newAmount)
... .Text = ... newAmount
end
There are hundreds of items in your inventory or backpack or whatever GUI and every single one of them has this connection.
The idea here is kind of just looking for what can be done to tweak this to something else, if there is a way to do so. If there is no way around this and its just “how its supposed to be”, does this have any downsides or is there some threshold where this’d cause some lag or memory leak or whatever?
Here is an example of this happening in my actual code:
local function AddItem(newItem)
-- Locate the newItem inside of GameData
local ItemData = GameData.ItemsData[newItem.Name]
local ItemType = ItemData.Type
--print(ItemData.Name, ItemType)
local InventoryMaxCap = CalculateInventory()
-- Create new gui instance relating to it
local newItemGUI = InventoryTakeoutButtonGUI:Clone()
newItemGUI.Name = ItemData.Name
newItemGUI.BackgroundColor3 = GameData.TypesData[ItemType].TypeColor
-- this is the thing, you just do categories like JobItems are types FISH, FRAGS, ORES etc. all and the rest are .Visible = false'd when the code will run
-- in the future but for now i dont care
local itemColor = ItemData.ToolModel.Handle.Color
local H,S,V = itemColor:ToHSV()
newItemGUI.TextColor3 = itemColor
newItemGUI.ContextualUIStroke.Color = Color3.fromHSV(H,S,V-0.3) -- i need to HSV this bro
-- the text
newItemGUI.Text = `[x{newItem.Value}] {ItemData.Name}` -- could use ItemData.Name or newItem.Name
newItemGUI.Parent = MapInventory
MapInventoryCapTitle.SurfaceGui.TextLabel.Text = `INVENTORY: {CalculateCurrentInventory()}/{CalculateInventory()}`
newItem.Changed:Connect(function() -- its an integer value
if newItem.Value > 0 then
newItemGUI.Text = `[x{newItem.Value}] {ItemData.Name}`
MapInventoryCapTitle.SurfaceGui.TextLabel.Text = `INVENTORY: {CalculateCurrentInventory()}/{CalculateInventory()}`
else
newItemGUI:Destroy()
MapInventoryCapTitle.SurfaceGui.TextLabel.Text = `INVENTORY: {CalculateCurrentInventory()}/{CalculateInventory()}`
end
end)
newItemGUI.MouseButton1Click:Connect(function()
-- open the gui (This can tweaked to be more versatile later just open it dude)
ItemTakeoutMenu.ItemName.Text = newItem.Name
ItemTakeoutMenu.ItemName.TextColor3 = newItemGUI.TextColor3
ItemTakeoutMenu.ItemShadow.Text = newItem.Name
ItemTakeoutMenu.ItemShadow.TextColor3 = newItemGUI.ContextualUIStroke.Color
local Request = ItemTakeoutMenu.Request -- textbox
Request.Text = ""
_G.OpenGUI(ItemTakeoutMenu)
confirmConnection = ItemTakeoutMenu.ConfirmButton.MouseButton1Click:Connect(function()
local numberRequest = tonumber(Request.Text)
if (tonumber(Request.Text) and tonumber(Request.Text) % 1 == 0 and tonumber(Request.Text) > 0) or string.lower(Request.Text) == "inf" then
if tonumber(Request.Text) < newItem.Value then
ItemTakeoutEvent:FireServer(Player.Backpack, Character, newItem.Name, tonumber(Request.Text), newItem)
elseif tonumber(Request.Text) >= newItem.Value then -- else works too but idc
ItemTakeoutEvent:FireServer(Player.Backpack, Character, newItem.Name, newItem.Value, newItem)
newItemGUI:Destroy()
elseif string.lower(Request.Text) == "inf" then
ItemTakeoutEvent:FireServer(Player.Backpack, Character, newItem.Name, newItem.Value, newItem)
newItemGUI:Destroy()
end
_G.CloseGUI(ItemTakeoutMenu)
Request.Text = ""
confirmConnection:Disconnect()
end
end)
cancelConnection = ItemTakeoutMenu.CancelButton.MouseButton1Click:Connect(function()
_G.CloseGUI(ItemTakeoutMenu)
Request.Text = ""
cancelConnection:Disconnect()
end)
--newItemGUI.Name
end)
end
There was a bug where my friend actually helped fix which was the confirmConnection and cancelConnection, which would start running multiple times everytime you’d Confirm and Cancel or do anything else. Every time you opened the menu again, clicked either button; It’d start running more and more of the same .MouseButton1Click since they were always there. Also sort of relevant to why I am asking this because these kind of things happen.
I assume any connection to a button or value will disconnect when that instance is destroyed, but what if they’re always there?
I don’t really know how to explain why I have this issue, every code till today worked using this and I didn’t encounter anything wrong with it, but it always bothered me that “Is this connection just always there checking for something to happen when 99% of the time nothing is happening to most of the things?”
