This code is terrible. Let's fix that, first.
Writing good code makes coding easier. It also means you write fewer bugs.
The first step is always tabbing properly. The next step is good variable names . You have both plr and playa -- neither is spelled correctly[^1], nor is plr accurate. Use character and player.
Next, use local variables. Otherwise things will act very unexpectedly[^2].
There's no reason to use hit.Parent["VechicleSeat"]. That exactly the same thing as hit.Parent.VehicleSeat.
pos = Vector3.new(0,1000,0)
function onTouch(hit)
if hit.Parent.VehicleSeat.SeatWeld then
for _, part in pairs (hit.Parent:GetChildren()) do
if part.Name == "paint" then
local gui = script.Parent.ScreenGui:Clone()
local character = (hit.Parent.VehicleSeat.SeatWeld.Part1.Parent)
local player = game.Players:FindFirstChild(character.Name)
gui.Parent = player.PlayerGui
hit.Parent.VehicleSeat.Disabled = true
hit.Anchored = true
character.Torso.Anchored = true
character:MoveTo(pos)
local cam = workspace.CurrentCamera
cam.FieldOfView = 100
cam.CameraSubject = hit.Parent
end
while wait() do
local thingy = gui.Color.ToolTip
print(thingy)
print(part)
part.BrickColor = BrickColor.new(Color3.new(thingy))
end
end
end
end
script.Parent.Touched:connect(onTouch)
You have improper error checking. If the object exists, then hit.Parent.VehicleSeat.SeatWeld will pass the if condition, so that if is pointless. If it doesn't exist, it will error. We need to use :FindFirstChild to see if objects exist. Since we would have to do it twice, it might be better to just use early returns instead of ifs -- or better yet, let's use functions!
In addition, your loop tries to change part's BrickColor even when it isn't necessarily a BasePart. You need to check for that.
Moreover, there's no reason to do stuff inside the loop that don't care about part. That means you should cut out the things about gui and cam -- in fact, you're just doing it if there is a child named "paint" does that sound like a use for :FindFirstChild rather than a loop?
function doThing(hit, weld)
if hit.Parent:FindFirstChild("paint") then
local gui = script.Parent.ScreenGui:Clone()
local character = weld.Part1.Parent
local player = game.Players:FindFirstChild(character.Name)
gui.Parent = player.PlayerGui
hit.Parent.VehicleSeat.Disabled = true
hit.Anchored = true
character.Torso.Anchored = true
character:MoveTo(pos)
local cam = workspace.CurrentCamera
cam.FieldOfView = 100
cam.CameraSubject = hit.Parent
for _, part in pairs (hit.Parent:GetChildren()) do
if part:IsA("BasePart") then
while wait() do
local thingy = gui.Color.ToolTip
print(thingy)
print(part)
part.BrickColor = BrickColor.new(Color3.new(thingy))
end
end
end
end
end
function onTouch(hit)
local vehicleSeat = hit.Parent:FindFirstChild("VehicleSeat")
if vehicleSeat then
local weld = vehicleSeat:FindFirstChild("SeatWeld")
if weld then
doThing(hit, weld)
end
end
end
script.Parent.Touched:connect(onTouch)
Note that you have a while loop that will never stop in the inside of a for loop. That means only the first thing in the for loop will ever get considered.
If you want to just change the color of the part to the color of gui.Color.ToolTip, you should use a Changed event. Either that, or spawn a new thread (but that's messier and slower)
pos = Vector3.new(0,1000,0)
function doThing(hit, weld)
if hit.Parent:FindFirstChild("paint") then
local gui = script.Parent.ScreenGui:Clone()
local character = weld.Part1.Parent
local player = game.Players:FindFirstChild(character.Name)
gui.Parent = player.PlayerGui
hit.Parent.VehicleSeat.Disabled = true
hit.Anchored = true
character.Torso.Anchored = true
character:MoveTo(pos)
local cam = workspace.CurrentCamera
cam.FieldOfView = 100
cam.CameraSubject = hit.Parent
gui.Color.Changed:connect( property )
if property == "ToolTip" then
local color = BrickColor.new( Color3.new( gui.Color.ToolTip ) )
for _, part in pairs(hit.Parent:GetChildren()) do
if part:IsA("BasePart") then
part.BrickColor = color
end
end
end
end
end
end
function onTouch(hit)
local vehicleSeat = hit.Parent:FindFirstChild("VehicleSeat")
if vehicleSeat then
local weld = vehicleSeat:FindFirstChild("SeatWeld")
if weld then
doThing(hit, weld)
end
end
end
script.Parent.Touched:connect(onTouch)
These changes make it easier to read -- which makes it easier to write. It also reduces the bugs, because it's simpler. We've eliminated a loop, reduced levels of indentation by a lot, shortened the main function, etc.
[^1]: It's almost always better to just use the full name for an object. If you save that much typing from using plr instead of player, you're very likely doing something wrong. It's hard to remember which incorrect spelling to use; it's easy to remember which correct spelling to use; it's also less cryptic and more readable. If you come to someone else for help with a variable named p, how are they to know it's supposed to be a player? How will they catch your mistake if you accidentally made it not be a player?
[^2]: In particular, you made gui be global. However, you have a loop which uses gui that keeps on going. That means new touches will "hijack" the old ones, which is probably not what you wanted (I doubt you wanted this since, for instance, you don't delete the old gui)
1 comment
To me, guessing, it may be that the 'while' loop on line 22 is stopping the 'for' loop from executing to the next 'BasePart' type Instance.