Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
0

Car Not Painting After Colour Selection?

Asked by 6 years ago

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)

1 answer

Log in to vote
0
Answered by 6 years ago

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 .Parents, 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:

  • If this script runs, then the player modifies 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
  • Your function doesn't use Cars (at least in the code you've given)
  • Line 22, you get another set of children and call it 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.)
  • I suspect that you have this script copy/pasted a bunch of times (one for each car). Instead of doing that, it would be easier to maintain if you only had one of these scripts, where it manages a list of button presses instead.
  • connect is deprecated; use Connect instead
  • You could use workspace 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.

0
Thank you! This has helped a lot and I'll be sure to keep these features in mind. I got my Cousin to help me with the scripts too as he fixed problems with the flag implementation in which I was having difficulties also. I'll try this out tomorrow and I can't wait to see if this has fixed it. Thank you for your time, knowledge and also answer. Definitely helped me out a lot here. RedVulcan17 2 — 6y
0
You're welcome. Let me know if you run into further trouble you can't figure out. chess123mate 5873 — 6y
0
It worked! Thank you so much for your help! I finally got the cars to paint plus you made it a whole lot easier with the RCar relying on IsCar to find out what car needs to be cloned. Definitely cut the script down a lot and has simplified it so it's even more easy to read and understand. Many thanks! RedVulcan17 2 — 6y
0
Glad to hear it! chess123mate 5873 — 6y
Ad

Answer this question