Now I must warn you this is a lot to understand and take in. The code itself is very big as it copies cars in ServerStorage to Workspace. I have recently added a new flag feature to expand more customization options in the game I'm making and I believe it has interfered with the car colouring. If anyone could analyse what the problem is and if you have any solutions please post them. The flag selector works but the colour palette doesn't. Thanks guys!
colour = script.Parent.Image colour2 = script.Parent.Parent.Parent.Colour.Selected.Value function SpawnCar(Cars) if script.Parent.Parent.IsCar.Value == 1 then local NewCar = game.ServerStorage.RCar1:Clone() print ("Is car 1 and cloned car 1") local children = NewCar.Body:GetChildren() for _, child in ipairs(children) do if child.Name == "Flag" then child.Decal.Texture = colour end end print ("Finished colouring") NewCar.Parent = game.Workspace NewCar:MakeJoints() print ("Put flag on car") script.Parent.Parent.Visible = false script.Parent.Parent.Parent.Frame.Visible = false script.Parent.Parent.Parent.Colour.Visible = false print ("Final touches..") local children4 = NewCar.Body:GetChildren() for _, child in ipairs(children4) do if child.Name == "Paint" then child.BrickColor = BrickColor.new(colour2) end end end end script.Parent.MouseButton1Click:connect(SpawnCar)
Using proper indentation and using variable names instead of script.Parent.Parent. ...
can help a lot. You can't know just by looking at the script what's wrong or not when you see a line with several .Parent
s, but if at the beginning you say things like car = script.Parent.Parent
and then later say car.IsCar.Value == 1
, then you're more likely to know if it's reasonable. (It's also easier to follow the logic.)
You use IsCar.Value to determine which car to clone? If your script can treat all cars the same (ex they all have a .Body), you can do something like this instead:
local newCar = game.ServerStorage["RCar" .. IsCar.Value]:Clone()
A few other problems:
script.Parent.Image
or script.Parent.Parent.Parent.Colour.Selected.Value
, your script will still be using the old values. Instead, assign to colour
and colour2
within SpawnCar
Cars
(at least in the code you've given)children4
, yet children
should still be perfectly fine. (Alternatively, you could put lines 24-26 in with the first for loop, or even merge it with the first if
using elseif
.)connect
is deprecated; use Connect
insteadworkspace
instead of game.Workspace
(but this is optional)None of the above is likely to fix the problem you're having (although fixing those things will improve your code). To debug, you're going to need to use more print
statements. ex, you suggest that the "Paint" section (lines 23-28) isn't working? Take a guess at what isn't working. ex, maybe children4
is empty? So after you get children4, print(#children4)
. Now, it's probably working, so it'll print some positive number. Good - now you know that the for
loop is running, so see if any of the children have a name of Paint
. Before the if
statement, perhaps print(child.Name, child.Name == "Paint")
, or perhaps within the if
statement simply have print("Child named paint, changing colour to", BrickColor.new(colour2))
. Keep doing this to figure out what is and isn't working; before long you'll find out exactly what's wrong.
Example of improved variable names (based on my guess of what everything is):
local textureLabel = script.Parent local carFrame = image.Parent local outerFrame = carFrame.Parent local colourObject = outerFrame.Colour.Selected function SpawnCar() -- TODO: If IsCar.Value might refer to something that isn't a car, check for that here and return early if needed local newCar = game.ServerStorage["RCar" .. carFrame.IsCar.Value]:Clone() local children = newCar.Body:GetChildren() local colour = BrickColor.new(colourObject.Value) for _, child in ipairs(children) do if child.Name == "Flag" then child.Decal.Texture = textureLabel.Image elseif child.Name == "Paint" then child.BrickColor = colour end end newCar.Parent = workspace newCar:MakeJoints() carFrame.Visible = false outerFrame.Frame.Visible = false outerFrame.Colour.Visible = false end script.Parent.MouseButton1Click:connect(SpawnCar)
Now, say you only wanted one of these scripts instead of a bunch of them (assuming you have more than one). The way to do this is to use get a table of all the buttons you want to spawn cars and to connect each one of them to the desired function. The LocalScript would then probably be a child of outerFrame
(in the script rewrite I suggested) instead of a child of textureLabel
. If we assume that every child of outerFrame
is a carFrame
(let's pretend it's named "CarFrame") and that each carFrame has that textureLabel (and let's pretend it's called "CarButton"), then:
local outerFrame = script.Parent local colourObject = outerFrame.Colour.Selected function SpawnCar(button) local carFrame = button.Parent local newCar = game.ServerStorage["RCar" .. carFrame.IsCar.Value]:Clone() local children = newCar.Body:GetChildren() local colour = BrickColor.new(colourObject.Value) for _, child in ipairs(children) do if child.Name == "Flag" then child.Decal.Texture = button.Image elseif child.Name == "Paint" then child.BrickColor = colour end end newCar.Parent = workspace newCar:MakeJoints() carFrame.Visible = false outerFrame.Frame.Visible = false outerFrame.Colour.Visible = false end local ch = script.Parent:GetChildren() for i = 1, #ch do if ch[i].Name == "CarFrame" then local button = ch[i].CarButton button.MouseButton1Click:Connect(function() SpawnCar(button) end) end end
This for loop fixes the problem of having to copy/paste the script for each car button by finding each button through-out your gui and making a connection to call SpawnCar
.