Part touched not giving new clothes

Help and Feedback Scripting Support
,
1

I made a part that gives the player a new outfit when the player touches it. For some reason it turns my player naked. What am I doing wrong?

shirtid = "9383189840"
pantsid = "9374031751"
button = script.Parent

function onTouch(part)

    if part.Parent:FindFirstChild("Shirt") then
        part.Parent.Shirt:Destroy()
    end

    if part.Parent:FindFirstChild("Pants") then
        part.Parent.Pants:Destroy()
    end

    local s = Instance.new("Shirt", part.Parent) 
    local p = Instance.new("Pants", part.Parent)
    s.Name = "Shirt"
    p.Name = "Pants"
    s.ShirtTemplate = "http://www.roblox.com/asset/?id="..shirtid 
    p.PantsTemplate = "http://www.roblox.com/asset/?id="..pantsid
end

button.Touched:connect(onTouch)
2

Are shirts visible with correct id?

3 
local s = Instance.new("Shirt") 
local p = Instance.new("Pants")
s.Parent = part.Parent
p.Parent = part.Parent

see if that fixes the issue.

nvm, thats actually correct what you did, I just never really see anyone do that. Though, you probably are incorrectly accessing the character or something and are parenting it to some body part or something.

4

Without being visible. When I switch to client and check my player’s shirt and pants it shows the ID correctly.

5

I believe it is accessing the character correctly because when I run the game and check the ids of the shirt and pants it’s updated to the one I put in the script.

6

Your problem is not with the code, but with the clothes themselves. That is, your clothes just have the wrong ID.
by the way heres refactor of your code

local button = script.Parent

local SHIRT_ID = "9383189826"
local PANTS_ID = "9374031732"

local function changePlayerClothing (part: BasePart)
	local character: Model | any = part.Parent
	if not ( character:IsA("Model") and character:FindFirstChildOfClass("Humanoid") ) then return end
	
	local playerShirt = part.Parent:FindFirstChildOfClass("Shirt")
	local playerPants = part.Parent:FindFirstChildOfClass("Pants")
	
	if playerShirt then playerShirt:Destroy() end
	if playerPants then playerPants:Destroy() end

	local newShirt = Instance.new("Shirt"); do
		newShirt.Parent = character
		newShirt.Name = "Shirt"
	end
	
	local newPants = Instance.new("Pants"); do
		newPants.Parent = character
		newPants.Name = "Pants"
	end
	
	newShirt.ShirtTemplate = `http://www.roblox.com/asset/?id={SHIRT_ID}`
	newPants.PantsTemplate = `http://www.roblox.com/asset/?id={PANTS_ID}`
end

button.Touched:connect(changePlayerClothing)

as the person at the bottom wrote, the problem is that the id in the directory and the actual id are different. This code solves your problem.

3 Likes
7

yea, whats happening is you are using the id on the catalog rather than the actual id of the shirt/pants.

you can paste the id into a shirt or pants in studio and then it will automatically format it for you using the correct id.

2 Likes
8

That did it! thank you!! :slight_smile:

9

I just want to add that

You could just use the Shirt and Pants objects that are already there, rather than destroying them and making new ones each time. It would also help in the long run to introduce some kind of tag so it doesn’t keep running if you’re already wearing the new outfit.

local ShirtID = shirt_id
local PantsID = pants_id

local Button = script.Parent
local RbxUrl = "http://www.roblox.com/asset/?id="

local function GiveNewClothingToPlayer(Hit)
	if Hit.Parent:FindFirstChild("Humanoid") == nil or Hit.Parent:FindFirstChild("HAS_CLOTHES") ~= nil then
		return
	end
	Instance.new("ObjectValue",Hit.Parent).Name = "HAS_CLOTHES"

	local PlayerShirt = Hit.Parent:FindFirstChildOfClass("Shirt")
	local PlayerPants = Hit.Parent:FindFirstChildOfClass("Pants")

	if PlayerShirt == nil then
		PlayerShirt = Instance.new("Shirt",Hit.Parent)
	end
	if PlayerPants == nil then
		PlayerPants = Instance.new("Pants",Hit.Parent)
	end

	PlayerShirt.ShirtTemplate = RbxUrl .. ShirtID
	PlayerPants.PantsTemplate = RbxUrl .. PantsID
end

Button.Touched:Connect(GiveNewClothingToPlayer)

This new code should help save on resources.

10

dude what. This code is literally meaningless in the context of its problem, which is completely unrelated to the colosseum of names. Moreover, I do not know why you should litter it with objectValue when you can use CollectionService. What is the general meaning of this tag when you use FindFirstChildOfClass? Moreover, you are using a check for … == nil, not “not…”, you use the second argument instance.new, as well as ugly string concatenation which is a bad practice in these cases. this code is inefficient.

also u dont deleting old clothings – I’m sorry, this is my nonsense, I didn’t notice that you were just changing your ID.

11

The problem was solved, and thus wrote out code for further improvement upon what was already there. What is "unrelated to the colosseum of names" even supposed to mean? Just sounds like some strange justification for me not to give my two-cents in the thread.

Old habit from 2013. It’s not that deep.

I had already answered that.

…

This just sounds like a subjective opinion being pushed as objective.

There is nothing wrong with comparing values to nil and getting a true / false boolean from that, rather than relying on the behavior that an object will be truthy and nil being falsey.

Furthermore, I fail to see how concatenation is bad practice. There is probably faster alternatives, but to say concatting is bad practice or inefficient is…strange.

I completely disagree that the code I had written out is inefficient. I can agree that the tag I used is, due to it being an older practice, and the same could be said for using the second argument for Instance.new (which this code would only run once if necessary), but none of the other methods I had used are objectively inefficient.

That was literally the point of the code I wrote

It’s not necessary to keep destroying and creating new clothes when you can simply re-use the ones present.

12

in fact, you can, but firstly it’s faster, and secondly it looks more concise.
image

Yes, I’m a stupid animal, you did everything right.

Yes, in fact concatenation is worse than interpolation, at least in terms of readability and typing convenience.

also, the second argument instance.new is slower than .parent (and it really pisses me off why we have to make our code worse because “roblox doesn’t want to change a couple of lines of code in its library”)

I still don’t understand the point of using HAS_CLOTHES. This is strange. There is no type. Why can’t you do something like a simple

local pants = Hit.Parent:FindFirstChildOfClass("Pants")
if not pants then pants = instance end
if not shirt then shirt = instance end
change id

is it really planned for the sake of a simple early exit from the function?

13

That is so negligible I fail to see the point in using that as an argument.

Again, this just sounds like a subjective opinion being pushed as objective.

Since it will only run that in the event that PlayerShirt or PlayerPants (or both) is nil (meaning either one or both are not present in the character), I don’t see the glaring issue. Using the second parameter is not the best practice I can agree, but it’s negligible in this instance.

…

…

Yes, it is. That’s the point.

Rather than having the code keep checking to find the clothing and then changing the templates over-and-over again every time Touched fires, it’s best to check if it was already applied, especially since in the original code the OP – including yourself – are destroying and making new clothes each time it runs.

Is it really that difficult to comprehend why I chose to introduce that as a solution?

Imagine you use a towel for drying your hands after you’re finished washing them. Would you A) re-use that towel until you feel it needs to be thrown into the washing machine which then it’s ready for use again, or B) discard it into the bin and order a new towel off Amazon for the next time you want to wash your hands, every time you want to wash your hands?

The latter is exactly how the OP and yourself wrote your program. Mine does the former.

1 Like
14

Before I read this, I was outweighing the meaninglessness of this idea. But you explained it to me, thank you)

15

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.