Tab your code correctly.
Use generic for loops: i is even reused twice! But it's not used anywhere except to index the thing your'e looping over.
MouseClick tells you playerWhoClicked. Don't use that loop. It's unnecessary.
Use meaningful variable names f is a character. s is shirt. p is pants
Miscellaneous: Use :Destroy() not :remove().
function onClicked(player)
local name = player.Name
local clothing = player:GetChildren()
for _, cloth in pairs(clothing) do
if cloth:IsA("Clothing") then
local character = game.Workspace:FindFirstChild(name)
local shirt = character:FindFirstChild("Shirt")
local pants = character:FindFirstChild("Pants")
local newShirt = script.Parent.Parent.Parent.Clothes.Shirt:clone()
local newPants = script.Parent.Parent.Parent.Clothes.Pants:clone()
shirt.Parent = script.Parent.Parent.PlayerStorage
pants.Parent = script.Parent.Parent.PlayerStorage
wait()
newShirt.Parent = character
newPants.Parent = character
wait(5)
newShirt:Destroy()
newPants:Destroy()
shirt.Parent = character
pants.Parent = character
end
end
end
script.Parent.ClickDetector.MouseClick:connect(onClicked)
Issue 1: character is being defined strangely, and in a weird place. It's the player's character, so just define it as player.Character (this also makes name unnecessary).
Issue 2: clothing = player:GetChildren() is wrong. The player doesn't have clothing, its character does. Use clothing = character:GetChildren().
Issue 3: cloth is unused. You should use cloth, you should not figure out shirt and pants.
Issue 4: The character may not have a Shirt or Pants. This will cause errors, since you do things with these objects anyways.
Get rid of shirts and pants. Just use cloth instead.
Issue 5: You don't give them new clothes per cloth they are wearing -- you do it once -- so it should not be in the loop.
Issue 6: You are using just one PlayerStorage, which now won't work necessarily, since all players using the stand would share it. We need privates ones: Just make a new one as a local model: local storage = Instance.new("Model")
function onClicked(player)
local character = player.Character
local clothing = character:GetChildren()
local storage = Instance.new("Model")
for _, cloth in pairs(clothing) do
if cloth:IsA("Clothing") then
cloth.Parent = storage
end
end
wait()
local newShirt = script.Parent.Parent.Parent.Clothes.Shirt:clone()
local newPants = script.Parent.Parent.Parent.Clothes.Pants:clone()
newShirt.Parent = character
newPants.Parent = character
wait(5)
newShirt:Destroy()
newPants:Destroy()
for _, cloth in pairs(storage:GetChildren()) do
cloth.Parent = character
end
end
script.Parent.ClickDetector.MouseClick:connect(onClicked)
Once again, this is fairly repetitive. You're reparenting clothes 4 times. Make a function to do that:
function moveClothes(from, to)
for _, child in pairs(from:GetChildren()) do
if child:IsA("Clothing") then
child.Parent = to
end
end
end
function onClicked(player)
local character = player.Character
local clothing = character:GetChildren()
local storage = Instance.new("Model")
moveClothes(character, storage);
wait()
moveClothes(script.Parent.Parent.Parent.Clothes:Clone(), character)
wait(5)
moveClothes(character, nil)
moveClothes(storage, character)
end
script.Parent.ClickDetector.MouseClick:connect(onClicked)
1 comment
This is once again terrible code (that AGAIN I have retyped a solution to fix) that has a LARGE number of issues. Address those first, then debug, THEN come back.