Why aren't part 1 and 2 getting destroyed?

can’t seem to figure out why they aren’t getting destroyed…
Edit: no output errors.

local runService = game:GetService("RunService")

game.ReplicatedStorage.floorPuffs.OnClientEvent:Connect(function(plrName, state)
	
	for i,v in pairs(game:GetService("Players"):GetChildren()) do
		
		if v.Name == plrName then
			
			local player = v
			
			local part1 = game.Lighting.ClientVFX.FloorPuffs:Clone()
			part1.Parent = player.Character
			part1.Name = "floorPuff1"
			
			local part2 = game.Lighting.ClientVFX.FloorPuffs:Clone()
			part2.Parent = player.Character
			part2.Name = "floorPuff2"
			
			if state == "Begin" then
				
				local function partToFeet()
					part1.Position = player.Character.HumanoidRootPart.Position + Vector3.new(-0.5,-2.5,0)
					part2.Position = player.Character.HumanoidRootPart.Position + Vector3.new(0.5,-2.5,0)
				end
				
				runService:BindToRenderStep("floorWind", 1, partToFeet)
				
			elseif state == "End" then
				
				part1:Destroy()

				part2:Destroy()
				
				runService:UnbindFromRenderStep("floorWind")
				
			end
		end	
	end
end)

Each time you declare Part1 and Part2 it’s local to the if statement’s scope, which that if statement is a block within the for loop, which is a block within the function connected to OnClientEvent. Every time that function gets called it’s starting a new function block not the same block.

What you’re actually seeing here is that if the “End” state is passed to the RemoteEvent then it’s deleting a new copy of Part1 and Part2, not ones that were previously declared. That thread has already finished executing so Part1 and Part2 are out of scope for subsequent handlers.

You’ll need to handle this system a different way. Particularly you’ll need some kind of upvalue container (be it a Janitor or CollectionService or whatever) that won’t be cleaned when the scope closes. That’d probably also be a great time to address some nitpicks:

  • Use GetPlayers to get a list of the players in the experience rather than GetChildren. Really this is just to keep your code idiomatic and predictable with methods given to you. For your case though you could just use FindFirstChild rather than iterating all players.

  • ipairs for arrays please. It’s pretty fast and good for iterating contiguous arrays. pairs is better for cases where you have arbitrary indices.

  • Lighting as a storage is an archaic practice. We have storage services for this purpose.

  • I don’t recommend binding a new function to RenderStepped every time a Begin state is passed to the remote, pretty sure that’ll override previous bindings or error. You may want to consider a pattern instead where BindToRenderStep is only updating active visual effects and the Begin/End state just adds or removes parts to be visualised.