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

Is there a better way of writing this code? Repeating same code?

Asked by
FiredDusk 1466 Moderation Voter
7 years ago

I feel as if there is a better way of wrong this code but making it in a shorter amount of lines. Any way?

home.MouseButton1Click:Connect(function()
    home.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= home then
            v.BackgroundColor3 = otherColor
        end
    end
end)

goRace.MouseButton1Click:Connect(function()
    goRace.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= goRace then
            v.BackgroundColor3 = otherColor
        end
    end
end)

cars.MouseButton1Click:Connect(function()
    cars.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= cars then
            v.BackgroundColor3 = otherColor
        end
    end
end)

tune.MouseButton1Click:Connect(function()
    tune.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= tune then
            v.BackgroundColor3 = otherColor
        end
    end
end)

customize.MouseButton1Click:Connect(function()
    customize.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= customize then
            v.BackgroundColor3 = otherColor
        end
    end
end)

options.MouseButton1Click:Connect(function()
    options.BackgroundColor3 = selectedColor
    for i,v in pairs(buttons) do
        if v ~= options then
            v.BackgroundColor3 = otherColor
        end
    end
end)
0
I would have a loop connecting all the buttons, then when any are clicked it calls the function, repeating those 6 lines. Should not take over 15 lines total iamnoamesa 674 — 7y

2 answers

Log in to vote
2
Answered by
jotslo 273 Moderation Voter
7 years ago
Edited 7 years ago

I thought I'd have a go at shortening the code as well. The code does exactly the same thing as yours, except it iterates through vars and puts everything into one loop, making it much shorter.

local vars = {home, goRace, cars, tune, customize, options}

for _, var in pairs(vars) do
    var.MouseButton1Click:Connect(function()
        var.BackgroundColor3 = selectedColor
        for i, v in pairs(var) do
            if v ~= options then
                v.BackgroundColor3 = otherColor
            end
        end)
    end)
end

0
Woah, I forgot you could even do this! FiredDusk 1466 — 7y
Ad
Log in to vote
1
Answered by 7 years ago

In my experience I find that it looks a lot better and stops some glitches when you reset all of the effects then set the effect you want. The most common problem is when effects overlap or duplicate when there should only be one effect, this both look bad and can be fixed simply.

As other people have commented you should be using a function for the reset:-

-- the vars you currently have
local selectedColor, otherColor = Color3.new(1,1,1), Color3.new(0,0,0) -- ect
local btns = {}

local function enblButton(btn)
    -- reset
    for i=1, #btns do
        btns[i].BackgroundColor3 = otherColor
    end

    -- enable the given button
    btn.BackgroundColor3 = selectedColor
end

-- we can now connect the button events as before

home.MouseButton1Click:Connect(function()
    enblButton(home) -- pass our button to the function 
end)


-- if you wanted you can also make a function to connect the events
local function conClick(btn, runFunction)
    btn.MouseButton1Click:Connect(function()
        enblButton(btn) -- pass our button to the function
        runFunction()
    end)
end

local function test()
    print('ran')
end
conClick(goRace, test) -- connects a new event for the given button with the function to run

I hope this helps.

Answer this question