FindFirstChild not working properly

Ok so I’ve been working on this game that sinks a ship but I don’t know what I did but FindFirstChild is no longer working anymore for me. I’ve already tried using pcall and printing but it never appears on the console I already checked if the script was disabled but it is not. I will be leaving the code below.

--LOCALS
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Event = ReplicatedStorage:WaitForChild("NotifcationEvent")

--FUNCTIONS
function notifcation(text)
	Event:FireAllClients(text)
end

function poweroff(a)
	wait(math.random(60, 120))
	if (a.className == "PointLight") or (a.className == "SpotLight") or (a.className == "SurfaceLight") then
		a:Destroy()
	end
	local b = a:GetChildren()
	for i = 1, #b do
		poweroff(b[i])
	end
	notifcation("The ship's lights went out.")
end

function respawn()
	local NewShip = game.ServerStorage.Model:Clone()
	local NewInvisWalls = game.ServerStorage.InvisWalls:Clone()
	NewShip.Parent = workspace
	NewInvisWalls.Parent = workspace
end

function sink()
	local Number = math.random(1, 2)
	local FrontSink = workspace.Model.FrontSinker
	local BackSink = workspace.Model.BackSinker
	if Number == 1 then
		FrontSink.Sink.Disabled = false
		notifcation("The ship is sinking in the bow.")
		poweroff(workspace.Model)
	elseif Number == 2 then
		BackSink.Sink.Disabled = false
		notifcation("The ship is sinking in the stern.")
		poweroff(workspace.Model)
	end
end
--

sink()

--RESPAWNING
spawn(function()
	while wait(1) do
		if workspace:FindFirstChild("Model") == nil then
			if workspace:FindFirstChild("InvisWalls") ~= nil then
				workspace.InvisWalls:Destroy()
			end
			respawn()
			local returned, data = pcall(function()
				local p = game.Players:GetChildren()
				for i = 1, #p do
					local head  = p[i].Character:FindFirstChild("Head")
					head:Destroy()
				end
			end)
			print(returned, data)
			sink()
		end
	end
end)

The function works properly. The issue is that those objects don’t exist where you are checking for them at the time you check for them, for whatever reason. I recommend you check your relevant code that either creates, parents, or destroys what you are checking for. If it used to work but no longer does, try to remember what recent changes, removals, or additions to those relevant actions you may have made.

I forgot to mention but that script is what runs the whole game. I rely on ROBLOX’s Physics and so the ship falls down into the void and gets deleted but, I’ll be sure to check what changes I made and see which one works and which one doesn’t.

Why not use IsA instead of comparing the ClassName? Btw, using parentheses in this case’s redundant.

Why not use a generic loop instead? Ex,

for _, v in ipairs(a:GetChildren()) do
    -- Code
end

~= nil is redundant, as the if statement will check whether a value’s true or not.

You should use GetPlayers instead of GetChildren, as GetPlayers will return a table with the players, vs where GetChildren will include non-player objs.

Lastly, I recommend giving your variable’s names that tell what they’re meant for. b for a's children? What do they mean/stand for? You should do like you did with the head variable; you can tell that it’s for the player’s head.

EDIT
Missed this, oops. : p
For the code part where it retrieve’s the player’s head, you’re not checking whether the head’s there or not, thus if the head’s not there, it’ll return nil, and when you go to destroy it, it’ll give an error along the lines of Attempt to call 'Destroy' on a nil value.

The solution’s to check whether it found the head or not via the if statement.

local PlayerHead = Players[i].Character and Players[i].Character:FindFirstChild('Head')
if PlayerHead then
   PlayerHead:Destroy()
end
1 Like

Ok, I’ll be sure to improve on it.

1 Like

I think a better use for this would be pairs.

in your code you use this

local p = game.Players:GetPlayers()
				for i = 1, #p do
					local head  = p[i].Character:FindFirstChild("Head")
					head:Destroy()
				end

but I think this would be better:

local p = game.Players:GetPlayers()
				for i, player in pairs(p) do
					local head  = player.Character:FindFirstChild("Head")
					head:Destroy()
				end

pairs will index all the current members of p without having to index them using the iterator (i)(in your code)

1 Like

That’s not how the generic for’s set up.
The correct way’s;

for _, v in ipairs(Players) do
    -- Code
end

Just like I pointed out in my previous reply, GetPlayer should be used because it’ll return the players in a table, vs with GetChildren where it’ll include non-player objs.

2 Likes

Yeah, I noticed it was going to error so I changed

for i = player, in pairs(p) do

to

for i , player in pairs(p) do
1 Like

sorry my example was sloppy, wasn’t accounting for best usage of getting the players

I was trying to make an example of using pairs, and yeah i just realized my typo, thanks. corrected it

1 Like

This thread should be moved to Code Review as FindFirstChild is in-fact working correctly but OP’s implementation of it was not correct and now this has turned into a code advice/improvement thread.

1 Like

The category guidelines for Code Review are fairly strict. Regardless of OP’s implementation of code, unworking code belongs in Scripting Support - Code Review is only for working code in which improvements, suggestions and so on are requested.

I’m not sure why this turned into an improvements thread, because a lot of these replies are off-topic. They should be focused on fixing the problem, not “this method is better than that”. That’s extraneous matters that can be tacked on with or after OP’s problem is solved.

As for @MySQL_Syntax (OP), try adding prints around your code (especially near the FindFirstChild part) and see what’s getting printed. This may help you find out why exactly FindFirstChild is not returning anything for you.