How can I improve my platform button code?

chrisby_bacon · October 24, 2021, 10:10pm

I’m pretty new to coding(took a long break, I was still new back then), and I’m wondering how I can improve this code? Please also explain what you’re doing and why.

if you want context, I’m making an obby for fun, it took me ~15 mins to make and script.

when this proximity prompt is triggered, these platforms appear(ignore the 2nd button at the end)

local PP = game.Workspace.Button1.ProximityPrompt
local Button1 = game.Workspace.Button1
local Baseplate = game.Workspace.Baseplate

local Platform1 = game.Workspace.Platform1
local Platform2 = game.Workspace.Platform2
local Platform3 = game.Workspace.Platform3
local Platform4 = game.Workspace.Platform4

PP.Triggered:Connect(function()
	Button1.BrickColor = BrickColor.Red()
	Platform1.Transparency = 0
	Platform2.Transparency = 0
	Platform3.Transparency = 0
	Platform4.Transparency = 0
	Platform1.CanCollide = true
	Platform2.CanCollide = true
	Platform3.CanCollide = true
	Platform4.CanCollide = true
	wait(6)
	Button1.BrickColor = BrickColor.Green()
	Platform1.Transparency = 1
	Platform2.Transparency = 1
	Platform3.Transparency = 1
	Platform4.Transparency = 1
	Platform1.CanCollide = false
	Platform2.CanCollide = false
	Platform3.CanCollide = false
	Platform4.CanCollide = false
end)

badexplorer

Rynappel · October 24, 2021, 10:15pm

Instead of manually setting each Platform’s transpercy, it’s better to use a loop to loop through each platform and set its properties. Like this,

Make a folder in the workspace called “Platforms” and put all your platforms in there so we can loop through the folder

local PP = game.Workspace.Button1.ProximityPrompt
local Button1 = game.Workspace.Button1
local Baseplate = game.Workspace.Baseplate

local Platforms = workspace.Platforms

PP.Triggered:Connect(function()
    Button1.BrickColor = BrickColor.Red()
    for _, Platform in pairs(Platforms:GetChildren() do -- loop through every platform
       Platform.Transparency = 0
       Platform.CanCollide = true
    end

	wait(6)

    Button1.BrickColor = BrickColor.Green()
    for _, Platform in pairs(Platforms:GetChildren() do -- loop through every platform
       Platform.Transparency = 1
       Platform.CanCollide = false
    end
end)

chrisby_bacon · October 24, 2021, 10:24pm

When I run this, I get this in the output:

  15:23:46.022  Workspace.Button1.ProximityPrompt.Platform Script:9: Expected ')' (to close '(' at column 26), got 'do'  -  Studio - Platform Script:9

iFlameyz · October 24, 2021, 10:26pm

add a “)” at the end of both "end"s

so as @Rynappel 's script

local PP = game.Workspace.Button1.ProximityPrompt
local Button1 = game.Workspace.Button1
local Baseplate = game.Workspace.Baseplate

local Platforms = workspace.Platforms

PP.Triggered:Connect(function()
    Button1.BrickColor = BrickColor.Red()
    for _, Platform in pairs(Platforms:GetChildren()) do -- loop through every platform
       Platform.Transparency = 0
       Platform.CanCollide = true
    end

	wait(6)

    Button1.BrickColor = BrickColor.Green()
    for _, Platform in pairs(Platforms:GetChildren()) do -- loop through every platform
       Platform.Transparency = 1
       Platform.CanCollide = false
    end
end)

chrisby_bacon · October 24, 2021, 11:15pm

same error happens unfortunately

  16:14:43.508  Workspace.Button1.ProximityPrompt.Platform Script:9: Expected ')' (to close '(' at column 26), got 'do'  -  Studio - Platform Script:9

chrisby_bacon · October 24, 2021, 11:21pm

yes, i did add a ‘)’ at the end of the end

Bunnyh_Script · October 24, 2021, 11:22pm

An ‘)’ is missing in the for loops. Try this

local PP = game.Workspace.Button1.ProximityPrompt
local Button1 = game.Workspace.Button1
local Baseplate = game.Workspace.Baseplate

local Platforms = workspace.Platforms

PP.Triggered:Connect(function()
    Button1.BrickColor = BrickColor.Red()
    for _, Platform in pairs(Platforms:GetChildren()) do
       Platform.Transparency = 0
       Platform.CanCollide = true
    end

	wait(6)

    Button1.BrickColor = BrickColor.Green()
    for _, Platform in pairs(Platforms:GetChildren()) do -- loop through every platform
       Platform.Transparency = 1
       Platform.CanCollide = false
    end
end)

SummerEquinox · October 24, 2021, 11:30pm

Further improving this, you can:

Use ipairs instead of pairs because you are iterating an array.
Use task library yield (task.wait(s)) as opposed to wait(), because wait() will likely see eventual deprecation.
Consistently access workspace or game.Workspace instead of alternating between the two, to improve code readability.
Connect every Triggered event to the same function, instead of building a function when the event triggers (may require debounce).

D0RYU · October 24, 2021, 11:55pm

local Button1 = workspace.Button1
local ProximityPrompt = Button1.ProximityPrompt
local Baseplate = workspace.Baseplate
local Platforms = workspace.Platforms

local Debounce = true

function SetPlatforms(Color, Transparency, CanCollide)
    Button1.BrickColor = BrickColor[Color]()

    for _, Platform in ipairs(Platforms:GetChildren()) do
        Platform.Transparency = Transparency
        Platform.CanCollide = CanCollide
    end
end

ProximityPrompt.Triggered:Connect(function()
    if Debounce then
        Debounce = false

        SetPlatforms("Red", 0, true)
        task.wait(6)
        SetPlatforms("Green", 1, false)

        Debounce = true
    end
end)

this code should be a good improvement from the solution

Rynappel · October 25, 2021, 2:12am

Oh yes, I forgot the ‘)’ lol I wasn’t writing it in studio properly lol

SummerEquinox · October 25, 2021, 12:51pm

This is incorrect. Wait global (alongside spawn and delay) is not deprecated and likely will not be for a while. The reason to not use it is because it will see deprecation in the future.

D0RYU · October 25, 2021, 4:12pm

they will be deprecated soon, but they aren’t deprecated yet