Can I make this code simpler or better in any way?

Help and Feedback Code Review
#1

Here, I have a lamp post that’s composed of 3 parts welded together. When a car hits it, the lamp post un anchors and falls down. Then, after a set amount of time, the lamp “respawns.”

local partA = script.Parent:FindFirstChild("Part A")
local partB = script.Parent:FindFirstChild("Part B")
local partC = script.Parent:FindFirstChild("Part C")

-- Define the original position and orientation of each part
local originalCFrame = partA.CFrame

function onTouched(hit)
	if hit.Parent.Name == "classic car" then
		print("Lamp Hit!")

		partA.Anchored = false
		partB.Anchored = false
		partC.Anchored = false

		wait(30)

		-- Reset the pole to its original position and orientation
		partA.CFrame = originalCFrame

		partA.Anchored = true
		partB.Anchored = true
		partC.Anchored = true
	end
end

partA.Touched:Connect(onTouched)
partB.Touched:Connect(onTouched)
partC.Touched:Connect(onTouched)

Is there anything I can do to improve the code? I’m still learning, so I’m guessing this is basic.

#2

Theres a small bug in your code. If the car hit the lamp, it falls to the ground and respawns after 30seconds, which is fine but when you hit the already hit lamp again it would glitch out as the function tries to make it collapse again. Add a variable called “wasHit”. In the very first line of the function you check if the variable is false. If not then escape via return. Example:

local wasHit = false

local function onTouched(hit)
       if (wasHit) then return end
       wasHit = true

       -- your code
   
       wasHit = false
end

Apart from that your code look good :+1: nice job

2 Likes
#3 
local partA = script.Parent:FindFirstChild("Part A")
local partB = script.Parent:FindFirstChild("Part B")
local partC = script.Parent:FindFirstChild("Part C")

-- Define the original position and orientation of each part
local originalCFrame = partA.CFrame

local parts = {partA, partB, partC}

function onTouched(hit)
  if hit.Parent:IsA("classic car") then 
    print("Lamp Hit!")

    -- Set the Anchored property of all parts to false
    for _, part in pairs(parts) do  -- Gets rid of Repetition
      part.Anchored = false
    end

    wait(30)

    -- Reset the pole to its original position and orientation
    partA.CFrame = originalCFrame

    -- Set the anchored property of all parts to true
    for _, part in pairs(parts) do -- Gets rid of Repetition
      part.Anchored = true
    end
  end
end

partA.Touched:Connect(onTouched)
partB.Touched:Connect(onTouched)
partC.Touched:Connect(onTouched)
#4 
local partA = script.Parent:FindFirstChildWhichIsA("Part")
local partB = script.Parent:FindFirstChildWhichIsA("Part")
local partC = script.Parent:FindFirstChildWhichIsA("Part")

-- Define the original position and orientation of each part
local originalCFrame = partA.CFrame

function onTouched(hit)
	if hit.Parent.Name == "classic car" then
		print("Lamp Hit!")

		-- Transfer ownership of the parts to the client temporarily
		partA:SetNetworkOwnership(game.Players.LocalPlayer)
		partB:SetNetworkOwnership(game.Players.LocalPlayer)
		partC:SetNetworkOwnership(game.Players.LocalPlayer)

		local startTime = tick()
		local delay = 30 -- delay in seconds

		while true do
			if tick() - startTime >= delay then
				break
			end
		end

		-- Reset the pole to its original position and orientation
		partA.CFrame = originalCFrame

		-- Transfer ownership of the parts back to the server
		partA:SetNetworkOwnership(game.Players.ServerPlayer)
		partB:SetNetworkOwnership(game.Players.ServerPlayer)
		partC:SetNetworkOwnership(game.Players.ServerPlayer)
	end
end

partA.Touched:Connect(onTouched)
#5

Won’t this risk it finding the same part 3 times?

1 Like
#6

Personally I’d store the parts in an array (or tables I think it’s called in lua) and then loop through it and make the parts’ anchored false/true

1 Like
#7

This is really not a good idea, for example if it’s a model with a lot of parts and op just wants the ones called Part A, Part B and Part C then this code will get random parts that aren’t the ones op wanted. Or maybe it’ll even just get the same part 3 times.