I'm making a character creator, but I feel like it has too many else ifs. I was wondering if there was anything that I could use to clean this up and make it easier to read/edit:
wait(1) local Player = game.Players.LocalPlayer local Shirt = 0 local Pants = 0 ---------------------------------------------------------------------------------------------------------------------------- script.Parent.Frame.ShirtButton.MouseButton1Click:Connect(function() if Shirt == 0 then Shirt = 1 game.ReplicatedStorage.CharItems.Shirt1:Clone().Parent = workspace.You elseif Shirt == 1 then Shirt = 2 workspace.You.Shirt1:Destroy() game.ReplicatedStorage.CharItems.Shirt2:Clone().Parent = workspace.You elseif Shirt == 2 then Shirt = 3 workspace.You.Shirt2:Destroy() game.ReplicatedStorage.CharItems.Shirt3:Clone().Parent = workspace.You elseif Shirt == 3 then Shirt = 4 workspace.You.Shirt3:Destroy() game.ReplicatedStorage.CharItems.Shirt4:Clone().Parent = workspace.You elseif Shirt == 4 then workspace.You.Shirt4:Destroy() Shirt = 1 game.ReplicatedStorage.CharItems.Shirt1:Clone().Parent = workspace.You end end) ------------------------------------------------------------------------------------------------------------ ---------------------------------------------------------------------------------------------------------------------------- script.Parent.Frame.PantsButton.MouseButton1Click:Connect(function() if Pants == 0 then Pants = 1 game.ReplicatedStorage.CharItems.Pants1:Clone().Parent = workspace.You elseif Pants == 1 then Pants = 2 workspace.You.Pants1:Destroy() game.ReplicatedStorage.CharItems.Pants2:Clone().Parent = workspace.You elseif Pants == 2 then Pants = 3 workspace.You.Pants2:Destroy() game.ReplicatedStorage.CharItems.Pants3:Clone().Parent = workspace.You elseif Pants == 3 then Pants = 4 workspace.You.Pants3:Destroy() game.ReplicatedStorage.CharItems.Pants4:Clone().Parent = workspace.You elseif Pants == 4 then workspace.You.Pants4:Destroy() Pants = 1 game.ReplicatedStorage.CharItems.Pants1:Clone().Parent = workspace.You end end) ------------------------------------------------------------------------------------------------------------
Thanks for the help
Improvements you can make to your code:
Pants
in your script) instead of using an if
statement to check it. In case you didn't know, %
is the modulus operator; it returns the remainder of a division. (You'll see it in use below.)wait(1)
at the top (better practice - the script becomes ready faster in most cases and if anything lags while loading then the script won't break)CharItems
.[]
notation. Since obj.Child
is the same as obj["Child"]
, we can access a child with a variable number at the end by doing obj["Name" .. num]
.Code so far (I don't comment it because I have better code below):
local player = game.Players.LocalPlayer local charItems = game.ReplicatedStorage:WaitForChild("CharItems") local frame = script.Parent:WaitForChild("Frame") local shirtNum = 0 local numShirts = 4 local shirt frame:WaitForChild("ShirtButton").MouseButton1Click:Connect(function() if shirt then shirt:Destroy() end shirtNum = shirtNum % numShirts + 1 shirt = charItems["Shirt" .. shirtNum]:Clone() shirt.Parent = player.Character end) local pantsNum = 0 local numPants = 4 local pants frame:WaitForChild("PantsButton").MouseButton1Click:Connect(function() if pants then pants:Destroy() end pantsNum = pantsNum % numPants + 1 pants = charItems["Pants" .. pantsNum]:Clone() pants.Parent = player.Character end)
Notice how the pants and shirt code are very similar? You can use a single function to handle any clothing type. This is especially valuable if you might add in other clothing types - if you ever need to make a change to all of them, you only need to do it in one place rather than 2+! However, it may not be the best idea for your case if:
"Shirt" .. theme .. num
, whereas pants just kept using "Pants" .. num
, or maybe one clothing type uses special permissions or checks that the others don't)The function HandleClothing
below does this, taking as parameter anything that is different between the types of clothing.
local player = game.Players.LocalPlayer local charItems = game.ReplicatedStorage:WaitForChild("CharItems") local frame = script.Parent:WaitForChild("Frame") function HandleClothing(buttonName, charItemName, maxItems) local num = 0 -- which clothing number we gave out last local item -- clothing item we last gave out frame:WaitForChild(buttonName).MouseButton1Click:Connect(function() if item then item:Destroy() end -- destroy the last item we gave out num = num % maxItems + 1 -- ex, if 'num' is 4 and 'maxItems' is 4, 4 % 4 is 0, so 'num' becomes 4%4+1 == 1 item = charItems[charItemName .. num]:Clone() item.Parent = player.Character end) end HandleClothing("ShirtButton", "Shirt", 4) HandleClothing("PantsButton", "Pants", 4)
Other potential improvements:
HandleClothing
function automatically detect the maximum number of items over time by using FindFirstChild
(and setting max
when it finds that a child doesn't exist), though this complicates the code a lot more just to save you from having to keep the number of clothing items up-to-date.