Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
✓ ANSWER ACCEPTED

Help with Clothing?

I have this script. It checks if a person has clothing when they join( because in some cases people don't have any ) and then, it changes the texture clothes. But, for some reason, it doesn't work. Can someone help?

function addClothing()
	if game.Players.LocalPlayer.Character:FindFirstChild('Pants') or game.Players.LocalPlayer.Character:FindFirstChild('Shirt') or game.Players.LocalPlayer.Character:FindFirstChild('Pants') and game.Players.LocalPlayer.Character:FindFirstChild('Shirt') then
		local clothing = game.Players.LocalPlayer.Character:GetChildren()
		for i, v in pairs(clothing) do
			if v.className == 'Shirt' then
				game.Players.LocalPlayer.Character:FindFirstChild('Shirt').ShirtTemplate = game.Players.LocalPlayer:FindFirstChild('BikeProperties').ShirtID.Value
				game.Players.LocalPlayer.Character:FindFirstChild('Pants').PantsTemplate = game.Players.LocalPlayer:FindFirstChild('BikeProperties').PantsID.Value
			end
		end
		for i, v in pairs(clothing) do
			if v.className == 'Pants' then
				game.Players.LocalPlayer.Character:FindFirstChild('Shirt').ShirtTemplate = game.Players.LocalPlayer:FindFirstChild('BikeProperties').ShirtID.Value
				game.Players.LocalPlayer.Character:FindFirstChild('Pants').PantsTemplate = game.Players.LocalPlayer:FindFirstChild('BikeProperties').PantsID.Value
			end
		end
	else
		local createPants = Instance.new('Pants')
		createPants.Parent = game.Players.LocalPlayer.Character
		createPants.Name = 'Pants'
		local createShirt = Instance.new('Shirt')
		createShirt.Parent = game.Players.LocalPlayer.Character
		createShirt.Name = 'Shirt'
		
	end
end

3 comments

0

Is it in a Server Script or a Local Script?

0

LocalScript

0

It spawns in your GUI when you join

1 answer

✓ Accepted answer

This script has a number of issues, mostly stylistically.


There is one main functional error:

This script doesn't ever call addClothing, so none of this would run.


You should definitely use variables to store things like game.Players.LocalPlayer.Character instead of having it written out each time.

Instead of referring to objects the way you are in your for loops, you should be using v. You don't know anything about the name of v from your current check. It also makes it significantly shorter.

In other news, don't use :FindFirstChild when you're going to just use . immediately after it. That will error if the child doesn't exist, just like if you used . instead of FindFirstChild so the benefit is defeated.


Your use of and and or at the top is really strange... or is not exclusive, so the final check for "and has pants" is completely unnecessary.

The entire if is really not what you want. You should just, before your loop, create a pants object if there wasn't one already, and create a shirt object if there wasn't one already (but these should be separate because they could have shirt and not pants, or vice versa).


You'll also want to remember that you should wait for the Character to load. LocalScripts run before the Character is ready.


local player = game.Players.LocalPlayer
repeat
	wait()
until player.Character
local character = player.Character

function addClothing()
	if not character:FindFirstChild("Pants") then
		Instance.new("Pants", character).Name = "Pants"
	end
	if not character:FindFirstChild("Shirt") then
		Instance.new("Shirt", character).Name = "Shirt"
	end
	-- Now character definitely has one Pants, Shirt object.
	local clothing = character:GetChildren()
	for _, child in pairs(clothing) do
		if child:IsA("Shirt") then
			child.ShirtTemplate = player.BikeProperties.ShirtID.Value
		elseif child:IsA("Pants") then
			child.PantsTemplate = player.BikeProperties.PantsID.Value
		end
	end
end

addClothing()

Then again, we expect the player (after the first check) to have exactly one Pants and exactly one Shirt. So we should just operate on those two children rather than use a loop at all:

local player = game.Players.LocalPlayer
repeat
	wait()
until player.Character
local character = player.Character

function addClothing()
	if not character:FindFirstChild("Pants") then
		Instance.new("Pants", character).Name = "Pants"
	end
	if not character:FindFirstChild("Shirt") then
		Instance.new("Shirt", character).Name = "Shirt"
	end
	-- Now character definitely has one Pants, Shirt object.
	character.Shirt.ShirtTemplate = player.BikeProperties.ShirtID.Value
	character.Pants.PantsTemplate = player.BikeProperties.PantsID.Value
end

addClothing()

Look at how much simpler the code is now! And it accomplishes exactly the same thing. Strive for this when programming -- simple, clear.

1388 characters -> 555 characters (60% shorter); 24 lines -> 16 lines (33% shorter).

A lot of programmers underestimate good style. I find that it's one of the main reasons why better programmers program better -- because they keep their programs easier to understand.

1 comment

0

Awesome! Thanks!

This discussion is preserved as it was. New answers, comments, and votes are closed.