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

Play solo doesn't work?

Asked by
iAviate 20
9 years ago

Hi,

I've basically just made a script that changes the color of lights when you click a switch.

There are two lights and one button. I'd play solo and it would all work fine, but then i'd play solo again just to make sure and all the lights would go haywire. It seems that every other time I play solo the scripts act up. Is that a problem on my end? If it is, i'll link a script in. -iaviate

-edit-

Script: every light has this script:

local screen = script.Parent.Parent.Parent.LYTE.Screen
local color = script.Parent

function onClicked() 
    if screen.BrickColor == BrickColor.Blue() then color.BrickColor = BrickColor.Blue() 
        script.Parent.PointLight.Enabled = true 
        script.Parent.PointLight.Color=Color3.new (46/225, 43/225, 149/225) -- Blue
    elseif screen.BrickColor == BrickColor.Green() then color.BrickColor = BrickColor.Green() 
        script.Parent.PointLight.Enabled = true 
        script.Parent.PointLight.Color=Color3.new (34/225, 97/225, 25/225) -- Green
    elseif screen.BrickColor == BrickColor.Red() then color.BrickColor = BrickColor.Red() 
        script.Parent.PointLight.Enabled = true
        script.Parent.PointLight.Color=Color3.new (207/225, 11/225, 0/225) -- Red
    elseif screen.BrickColor == BrickColor.Yellow() then color.BrickColor = BrickColor.Yellow() 
        script.Parent.PointLight.Enabled = true 
        script.Parent.PointLight.Color=Color3.new (236/225, 189/225, 34/225) -- Yellow
    elseif screen.BrickColor == BrickColor.White() then color.BrickColor = BrickColor.White() 
        script.Parent.PointLight.Enabled = true 
        script.Parent.PointLight.Color=Color3.new (255/225,255/225, 255/225) -- White
    elseif screen.BrickColor == BrickColor.Black() then color.BrickColor = BrickColor.Black()   
    script.Parent.PointLight.Enabled = false
end
end

screen.SurfaceGui.TextButton.MouseButton1Down:connect(onClicked)

The button (screen) has this:

local screen = script.Parent.Parent.Parent



function one()
    if screen.BrickColor == BrickColor.Blue() then screen.BrickColor = BrickColor.Green()
    elseif screen.BrickColor == BrickColor.Green() then screen.BrickColor = BrickColor.Red()
elseif screen.BrickColor == BrickColor.Red() then screen.BrickColor = BrickColor.Yellow() 
    elseif screen.BrickColor == BrickColor.Yellow() then screen.BrickColor = BrickColor.White() 
    elseif screen.BrickColor == BrickColor.White() then screen.BrickColor = BrickColor.Black()
        elseif screen.BrickColor == BrickColor.Black() then screen.BrickColor = BrickColor.Blue()   end
    end

screen.SurfaceGui.TextButton.MouseButton1Down:connect(one)

0
Include the script. As far as I know, ROBLOX doesn't have any bugs involving repeatedability, so you've most likely done something wrong. BlueTaslem 18071 — 9y
0
Updated. iAviate 20 — 9y

1 answer

Log in to vote
0
Answered by
BlueTaslem 18071 Moderation Voter Administrator Community Moderator Super Administrator
9 years ago

There's no reason for your code to be so complex!

Light Script

First, you use script.Parent.PointLight 11 times. Use a variable, say, light.


You continually have this pattern:

    if X == A then
        Y = A
    elseif X == B then
        Y = B
    elseif X == C then
        Y = C
    ...

Instead, just say before the ifs and only once:

    Y = X

that is,

    ....
    color.BrickColor = screen.BrickColor
    if screen.BrickColor == BrickColor.Blue() then
        ....

A similar thing can be said for enabling / disabling the light. Since only one disables the light, you might as well just default to enabling, and only changed Enabled for Black:

    light.Enabled = true
    if screen.BrickColor == BrickColor.Blue() then
        ....
    elseif screen.BrickColor = BrickColor.Black() then
        light.Enabled = false
    end

At this point, we still have a really large if .... elseif thing going on, which probably isn't what you want.

One option at this point would be to ask BrickColor for the Color3 values; these are different from what you've picked, but depending on why you picked these in particular, that might be okay.

That would make the entire function this:

function onClicked() 
    color.BrickColor = screen.BrickColor
    light.Color = screen.BrickColor.Color
    light.Enabled = true;
    if screen.BrickColor == BrickColor.Black() then
        light.Enabled = false
    end
end

In fact, you could shorten it a little by removing the if:

function onClicked() 
    color.BrickColor = screen.BrickColor
    light.Color = screen.BrickColor.Color
    light.Enabled = ( screen.BrickColor ~= BrickColor.Black() )
end

If you wanted custom colors, you could keep it how it is, or you could use a dictionary.

local color3s = {
    ["Bright blue"] = Color3.new (46/225, 43/225, 149/225),
    ["Dark green"] = Color3.new (34/225, 97/225, 25/225),
    ["Bright red"] = Color3.new (207/225, 11/225, 0/225),
    ["Bright yellow"] = Color3.new (236/225, 189/225, 34/225),
    ["White"] = Color3.new (255/225,255/225, 255/225)
};

function onClicked()
    color.BrickColor = screen.BrickColor
    light.Color = colors3s[ tostring( screen.BrickColor ) ]
    light.Enabled = ( screen.BrickColor ~= BrickColor.Black() )
end

Button Script

Again, we find ourselves repeating a lot. At the least, we could make a variable for screen.BrickColor:

local old, new = screen.BrickColor
if old == BrickColor.Blue() then
    new = BrickColor.Green()
elseif ....
end
screen.BrickColor = new

That's not really the best. What we're doing here is defining some sort of cycle, so we should just store it in a list and use a loop to figure out what's next. (We can also make it briefer by using the string names)

local cycle = {
    BrickColor.Blue(), BrickColor.Green(), BrickColor.Red(),
    BrickColor.Yellow(), BrickColor.White(), BrickColor.Black()}

function one()
    for i = 1, #cycle - 1 do -- Not black...
        if screen.BrickColor == cycle[i] then
            screen.BrickColor = cycle[i + 1] -- Next in cycle
            return -- I'm done; I found the match
        end
    end
    -- since black is last a simple + 1 won't be enough
    screen.BrickColor = cycle[1] -- First in cycle
end

We actually can pack up the last element into there pretty easily; just make a copy. Since we stop at the first one, it's okay to have duplicates:

local cycle = {
    BrickColor.Blue(), BrickColor.Green(), BrickColor.Red(),
    BrickColor.Yellow(), BrickColor.White(), BrickColor.Black()}
table.insert(cycle, cycle[1])

function one()
    for i = 1, #cycle do
        if screen.BrickColor == cycle[i] then
            screen.BrickColor = cycle[i + 1]
            return
        end
    end
end

Beyond This...

I don't see any particular issues with the code.

If there are problems, I'm guessing it has to do with objects not loading in the order that you expect / want them to.

Check your output, print everything that's suspicious.


You haven't said what exactly is wrong. What does act up mean?

Ad

Answer this question