Hi there! Hope you are having a good day. This has been bugging me for about an hour so I had to resort to the devforums. Basically, I have a pet system. It loops through the pet folder which is in the player.
local plr = game.Players.LocalPlayer
local Pets = plr:WaitForChild("Pets")
local template = script.Parent.Template
repeat
wait(1)
for _, v in pairs(Pets:GetChildren()) do
if v:IsA("IntValue") then
for i = 1, v.Value do
local new = template:Clone()
new.Name = tostring(v)
new.Text = tostring(v)
new.Parent = script.Parent
wait(.15)
end
end
end
until script.Disabled == true
When I changed the value of a Pet [Named 1] to have a value of 2 [in the players inventory], it will just keep duplicating the amount of textbutton to more than 2 even tho the player only has 2 in the inventory?
You should make this into a function instead of disabling the script. Until the script is disabled, it will infinitely add pets. Try this, it will update every frame:
local plr = game.Players.LocalPlayer
local Pets = plr:WaitForChild("Pets")
local template = script.Parent.Template
local function UpdatePets()
for _, v in pairs(script.Parent:GetChildren()) do
if v.ClassName == template.ClassName then
v:Destroy()
end
end
for _, v in pairs(Pets:GetChildren()) do
if v:IsA("IntValue") then
for i = 1, v.Value do
local new = template:Clone()
new.Name = tostring(v)
new.Text = tostring(v)
new.Parent = script.Parent
end
end
end
end
game:GetService([[RunStepped]]).RenderStepped:Connect(UpdatePets)
The Explanation
Every frame that RenderStepped runs (about 60 times a second), we want to update the slots.
We use the UpdatePets() function to accomplish this. First, we remove any objects that are the same type of object (If your template is a frame, it will remove the frames, if your template is a text label, it will remove the old text labels (ClassName is compared). Now that we have remove the old templates, we make the new ones based off of the inventory and templates.
The original problem you had was A) You were using disabling the script to run code (A function is better) and B) You repeated adding them every second, and your script never was disabled fast enough.
The cool thing about RenderStepped is it runs in a seperate thread so the print(1) would run in the renderstepped example, while with the while true do loop it would not.
Unless you have a check statement in there that doesn’t blanket destroy child objects and instead only adds new items or removes old items, running this every frame would be quite expensive. It’s also bad practice. Changes should only be made as in when necessary, which means if pets has a reason to update (addition, removal, queued refresh, etc).
e.g.
local function UpdatePets()
for _, PetBlock(Container:GetChildren()) do
if PetBlock.ClassName == template.ClassName then
if --[[player doesn't have pet but block exists]] then
PetBlock:Destroy()
end
end
end
for _, Pets in pairs(Container:GetChildren()) do
if --[[player has pet but block doesn't exist]] then
-- Create button
end
end
end
PetsContainer.AddedSignal:Connect(UpdatePets)
PetsContainer.RemovedSignal:Connect(UldatePets)
UpdatePets().
-- Above signals can be simplified to an operation queue
As @colbert2677 was saying, it would be better to update when the pets change. (It would help performance significantly, removing the destruction and addition of gui objects per second)
The following example can replace the render stepped.
Pets.Changed:Connect(UpdatePets)
Note: You would also want to call this function when you change the number of pets, so you could do this using a remote event also. Then, when you update the players pets, fire the player.
