Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
✓ ANSWER ACCEPTED

Why can't I get "v" to change colors?

I'll explain further about things if needed.

pos = Vector3.new(0,1000,0)
shop = Vector3.new(0,0,0)
gui = script.Parent.ScreenGui:Clone()

function onTouch(hit)
	if hit.Parent["VehicleSeat"]["SeatWeld"] then
	for i,v in pairs (hit.Parent:GetChildren()) do
	if v.Name == "paint" then
	gui = script.Parent.ScreenGui:Clone()
	plr = (hit.Parent.VehicleSeat.SeatWeld.Part1.Parent)
	playa = game.Players:FindFirstChild(plr.Name)
	gui.Parent = playa.PlayerGui
	hit.Parent.VehicleSeat.Disabled = true
	hit.Anchored = true
	plr.Torso.Anchored = true
	plr:MoveTo(pos)
	local cam = workspace.CurrentCamera
	cam.FieldOfView = 100
	cam.CameraSubject = hit.Parent
end

while wait() do
	thingy = gui.Color.ToolTip
	print (thingy)
	print (v)
	v.BrickColor = BrickColor.new(Color3.new(thingy))
	end
	end
	end
end

script.Parent.Touched:connect(onTouch)

1 comment

0

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.

1 answer

✓ Accepted answer

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)

This discussion is preserved as it was. New answers, comments, and votes are closed.