Weld script not working

I’ve got a weld script that is meant to weld a hi-vis vest to the player’s Torso, but when I click the part it doesnt clone or weld.

local player = game.Players.LocalPlayer
 
function onClicked(player)
    if not character or not character.Parent then
        character = player.CharacterAdded:wait()
    end
    if player:findFirstChild("Humanoid") ~= nil and character:findFirstChild("Vest") == nil then
        local g = script.Parent.Parent.Coathanger.Vest:clone()
        g.Parent = character
        local C = g:GetChildren()
        for i = 1, #C do
            if C[i].className == "Part" or C[i].className == "UnionOperation" or C[i].className == "MeshPart" then
                local W = Instance.new("Weld")
                W.Part0 = g.Middle
                W.Part1 = C[i]
                local CJ = CFrame.new(g.Middle.Position)
                local C0 = g.Middle.CFrame:inverse()*CJ
                local C1 = C[i].CFrame:inverse()*CJ
                W.C0 = C0
                W.C1 = C1
                W.Parent = g.Middle
            end
            local Y = Instance.new("Weld")
            Y.Part0 = character.Torso
            Y.Part1 = g.Middle
            Y.C0 = CFrame.new(0, 0, 0)
            Y.Parent = Y.Part0
        end
 
        local h = g:GetChildren()
        for i = 1, # h do
            if h[i].className == "Part" or h[i].className == "UnionOperation" or C[i].className == "MeshPart"  then
                h[i].Anchored = false
                h[i].CanCollide = false
            end
        end
    end
end
 
script.Parent.ClickDetector.MouseClick:Connect(onClicked)
 if player:findFirstChild("Humanoid") ~= nil and character:findFirstChild("Vest") == nil then

it is meant to be :FindFirstChild(“Humanoid”)

The capitalization of the f doesn’t change anything, it’s still a valid call.

You didn’t even define what the character variable is supposed to be

Here’s your issue:

A player object does not have a Humanoid object, it’s only found in Character Models/Rigs (Or by inserting one), also…

If this is on a server script then you cannot define the local player that way, just use the ClickDetector's MouseClick parameter
This should be the fixed script, you can use the print() statements to debug it

function onClicked(player)
    local character = player.Character
    if not character or not character.Parent then
        print("Waiting for a character to load?")
        character = player.CharacterAdded:wait()
    end
    if character:findFirstChild("Humanoid") ~= nil and character:findFirstChild("Vest") == nil then
        print("Creating the welds for the player")
        local g = script.Parent.Parent.Coathanger.Vest:clone()
        g.Parent = character
        local C = g:GetChildren()
        for i = 1, #C do
            if C[i].className == "Part" or C[i].className == "UnionOperation" or C[i].className == "MeshPart" then
                print("Adding a new weld")
                local W = Instance.new("Weld")
                W.Part0 = g.Middle
                W.Part1 = C[i]
                local CJ = CFrame.new(g.Middle.Position)
                local C0 = g.Middle.CFrame:inverse()*CJ
                local C1 = C[i].CFrame:inverse()*CJ
                W.C0 = C0
                W.C1 = C1
                W.Parent = g.Middle
            end
            local Y = Instance.new("Weld")
            Y.Part0 = character.Torso
            Y.Part1 = g.Middle
            Y.C0 = CFrame.new(0, 0, 0)
            Y.Parent = Y.Part0
        end
 
        local h = g:GetChildren()
        for i = 1, # h do
            if h[i].className == "Part" or h[i].className == "UnionOperation" or C[i].className == "MeshPart"  then
                h[i].Anchored = false
                h[i].CanCollide = false
            end
        end
    else
        print("Valid character not found!")
    end
end

script.Parent.ClickDetector.MouseClick:Connect(onClicked)
2 Likes

I had defined character before, but I removed it and I forgot to add it back in before posting this, I’ll test the code

The capitalisation of the f is actually pretty important. :findFirstChild is a deprecated method, you shouldn’t be using it anymore. Along with .className and :clone(). The newer methods/properties have the first letter capitalised (:FindFirstChild(), .ClassName, :Clone()) You can check if a method is deprecated by looking at the api reference for it.


local C = g:GetChildren()
    for i = 1, #C do

Please don’t use for loops to iterate over tables. Just use a pairs/ipairs loop. Using for loops harms readability when they are used incorrectly (like in this case). Debugging your code is going to be a nightmare if it’s impossible to read.

for _, part in pairs(g:GetChildren()) do

if C[i].className == "Part" or C[i].className == "UnionOperation" or C[i].className == "MeshPart" then

Here you can just use IsA (capital I, again; pretty important) to check if the object is a part.

if part:IsA("BasePart") then

function onClicked(player)

You should always declare functions as local, unless you’re overwriting a function or declaring a method (the same goes for variables). It doesn’t noticably impact performance, but is good practice.

local function onClicked(player)

Also consider using a WeldConstraint instead of a Weld so you don’t have to manually set the C0/C1 properties.

1 Like