2

The goal of this code is to spawn a ball "GlowyBall" in 1 of 5 preset locations randomly. This script activates when a player hits a button. The ball also needs to spawn as 1 of 3 colors randomly. The code works for the most part, but I am struggling when it comes to making this code optimized. I don't know which datatype I should or even can use to replace these if statements. I am just trying to learn different avenues that can be taken. The reason this code needs to be optimized is that it could be used thousands of times per minute, and I don't want the game to be held back by the code.

...

-- Says that there will be 3 colors
local ColorRange = 3

-- Says that there will be 5 spawn locations
local range = 5


-- Makes the code run continuously
while true do

    local ColorNumber = math.random(1, ColorRange)

    local Number = math.random(1, range)

    -- Chooses the random color
    if ColorNumber == 1 then
        game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball1.Color = Color3.new(1, 0, 0)
    end

    if ColorNumber == 2 then
        game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball2.Color = Color3.new(0, 1, 0)
    end

    if ColorNumber == 3 then
        game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball3.Color = Color3.new(0, 0, 1)
    end

    -- Chooses which ball will get cloned
    if Number == 1 then
        ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball1
    end

    if Number == 2 then
        ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball2
    end

    if Number == 3 then
        ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball3
    end

    if Number == 4 then
        ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball4
    end

    if Number == 5 then
        ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1.Glowyball5
    end

    wait(.6)
    local Clone = ClonePart:Clone()

    script.Parent.ClickDetector.MouseClick:connect(function()
    Clone.Parent = game.Workspace
    Clone.Anchored = false
    end)

end

...

I am fairly new to programming as a whole so feel free to teach me a few things, thanks.

Kylaaa
  • 6,349
  • 2
  • 16
  • 27
E Skees
  • 31
  • 4

2 Answers2

1

Instances in Roblox can be accessed in a few different ways; the most common is dot notation (eg. game.Workspace.Part). It's also possible to access instances like items in a table (eg. game["Workspace"]["Part"]). This is useful when accessing an instance with a space in its name, or to add something to the start/end of a string before accessing it.

The if statements for choosing which ball to clone can then be reduced to the following:

-- Chooses which ball will get cloned
ClonePart = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1["Glowyball" .. Number] -- Add Number to the end of "Glowyball" before accessing it

The same could be done with choosing the random color, as well as substituting multiple if statements for one if-elseif statement could make the code more readable.

-- Chooses the random color
local Glowyball = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1["Glowyball" .. ColorNumber]
if ColorNumber == 1 then
    Glowyball.Color = Color3.new(1, 0, 0)
elseif ColorNumber == 2 then
    Glowyball.Color = Color3.new(0, 1, 0)
elseif ColorNumber == 3 then
    Glowyball.Color = Color3.new(0, 0, 1)
end
Heliodex
  • 169
  • 2
  • 8
  • 1
    Thank you, I did actually think to concatenate the random values, but I guess I couldn't get the syntax correct. – E Skees Jul 31 '22 at 02:28
1

Heliodex did a good job explaining how to simplify finding the instances, but you could take it a step further and remove the if-statements entirely by replacing them with arrays.

Typically, from a general programming standpoint, when you find that you have a lot of very similar if-statements, the solution is to use a switch-statement instead of if-elseif chains. Switch statements allow you to define a bunch of possible cases and when it executes, it will jump specifically to the case that matches. However, neither lua nor luau support switch statements, but we can get the benefit of switch-statements.

Since you are using random numbers already, you can pre-define your colors into an array and have the random number simply pick an index to use...

-- make some colors to pick from
local colors = {
    Color3.new(1, 0, 0),
    Color3.new(0, 1, 0),
    Color3.new(0, 0, 1),
}

-- Set up the list of spawn locations
local locationGroup = game.ServerStorage.GlowyBallsSideA.GlowyBallGroup1
local spawnLocations = {
    locationGroup.Glowyball1,
    locationGroup.Glowyball2,
    locationGroup.Glowyball3,
    locationGroup.Glowyball4,
    locationGroup.Glowyball5,
}
-- could this be simplified to ...?
-- local spawnLocations = locationGroup:GetChildren()


-- when someone clicks the button, spawn the part
script.Parent.ClickDetector.MouseClick:Connect(function()
    -- choose a color and spawn location
    local colorNumber = math.random(1, #colors)
    local spawnNumber = math.random(1, #spawnLocations)
    local clonePart = spawnLocations[spawnNumber]
    local color = colors[colorNumber]

    -- spawn it
    local clone = clonePart:Clone()
    clone.Color = color
    clone.Parent = game.Workspace
    clone.Anchored = false
end)

The advantage of this approach is that you can simply add more colors and spawn locations to those arrays, and the rest of your code will still work.

Kylaaa
  • 6,349
  • 2
  • 16
  • 27