How do i make this script more efficient?

Help and Feedback Scripting Support
#1

So im trying to make this script run whenever a player has an item added to their custom inventory but im not sure how

local Player = game.Players.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")
local InventoryStuff = PlayerGui.MainGui:WaitForChild("InventoryHolder").Inventory

for index,child in pairs(InventoryStuff.Duplicates:GetChildren()) do
	if child:IsA("ImageButton") then
		print("IS an Imagebutton")
		local newchild = child:Clone()
		newchild.Parent = script.Parent
		newchild.MouseButton1Click:Connect(function()
			script.Parent.Frame.Visible = true
			script.Parent.Frame.Title.Text = "Are you sure you want to sell "..newchild.ArmorName.Value
		end)
		script.Parent.Frame.Sell.MouseButton1Click:Connect(function()
			newchild:Destroy()
			child:Destroy()
		end)
	end	
end
#2

Use ChildAdded and ChildRemoved events to only make changes when something gets added or removed. Then update your Gui accordingly.

1 Like
#3

Is it .ChildAdded or :ChildAdded? And would i make an if statement?

#4

.ChildAdded, also is your inventory managed by frames, or is the frames a copy of a server-saved inventory?

1 Like
#5

It’s managed by frames

#6

Would i add an if statement? if Inventory.childadded then?

1 Like
#7

Hmm so I don’t think this would work for your system but try it out.

Inventory.ChildAdded:Connect(function (child)
   print ("Removed "..child.Name)
end)

This was a suggestion for a physical object inventory in Replicated, but it might work. I recommend changing to a server-handled inventory so that players can’t exploit your game and give themselves any items.

#8

I tried on server right now but it doesn’t work, it doesn’t even print

game.Players.PlayerAdded:Connect(function(Player)
local PlayerGui = Player:WaitForChild("PlayerGui")
local InventoryStuff = PlayerGui:WaitForChild("MainGui"):WaitForChild("InventoryHolder").Inventory

InventoryStuff.Duplicates.ChildAdded:Connect(function()
	for index,child in pairs(InventoryStuff.Duplicates:GetChildren()) do
		if child:IsA("ImageButton") then
			print("IS an Imagebutton")
			local newchild = child:Clone()
				newchild.Parent = PlayerGui:WaitForChild("MainGui").SellFrame.Duplicates
				newchild.Visible = true
				newchild.MouseButton1Click:Connect(function()
				
				PlayerGui:WaitForChild("MainGui").Frame.Visible = true
				PlayerGui:WaitForChild("MainGui").Frame.Title.Text = "Are you sure you want to sell "..newchild.ArmorName.Value
			end)
			PlayerGui:WaitForChild("MainGui").Frame.Sell.MouseButton1Click:Connect(function()
				newchild:Destroy()
				child:Destroy()
				PlayerGui:WaitForChild("MainGui").Frame.Visible = false
			end)
		end	
	end
	end)
end)
#9

Server can’t access playerGui because they are local instances. You have to basically remake your system to work with the server. I would recommend taking the time to remake the system or simply use your old script. There is no need to iterate when using a function though as it’s redundant.

Inventory.ChildAdded:Connect(function (child)
   -- child is the newly added itemFrame in inventory
   child.MouseButton1Click:Connect(function()
      -- new connection here
   end)
end)

This is also means you’d have to add the actual child from elsewhere. Perhaps an external script that it’s purpose is solely to add the itemFrame into the PlayerGui.

Conclusion: You have asked for a more efficient script. I would say you could have a more efficient system.

Here is a script-handled inventory system. Could be a bit advanced but 100000% worth learning it.

ProfileService is also an open source tool for creating inventories and having them save in DataStores as well as take care of all the corner cases for data like autosaving, saving when server shut downs, and more stuff. Try it out as it will also save you the hassle of perfecting your inventory.

#10

I Already made an inventory system this is just a sell system and im trying to get the inventory of the player