Improvements for a Gui script that changes a part's colour when clicked

Help and Feedback Code Review
#1

So It’s been 8 day’s Since I’ve been learning Lua and I’ve learned quite a lot in those 8 days.
So I made a basic gui script that changes colour of an part and closes when clicked on a button
Here is the script

Script 
local player = game.Players.LocalPlayer
local playerGui = player:WaitForChild("PlayerGui")
local close = playerGui.ScreenGui.X
local gui = playerGui.ScreenGui
local Black = playerGui.ScreenGui.TextButton
local Blue = gui.TextButton2
local Orange = gui.Textbutton3
local Open = gui.Open

close.MouseButton1Click:Connect(function() --Close Button
	Black.Visible = false 
	Blue.Visible = false
	Orange.Visible = false
	print("closed")
	Open.Visible = true
	close.Visible = false
end)

Open.MouseButton1Click:Connect(function()-- Open Button
	Black.Visible = true 
	Blue.Visible = true
	Orange.Visible = true
	print("opened")
	Open.Visible = false
	close.Visible = true
end)

I know it can be improved so I would like to improve the code and make it smaller and I just need some help to improve it.
Thank you

2 Likes
#2

I don’t think this can be that much improved in my case since it looks really successful. Though you can make less variables to make it shorter.

2 Likes
#3

Oh if that’s the case the I’ll do my best to make less variables.T
Thank you Sir!

2 Likes
#4

#1 - This should probably be moved to #help-and-feedback:code-review

#2 - In the beginning, you define the ScreenGui and don’t use it at every time you could and should have. Additionally, a WaitForChild on the ScreenGui is safer to make sure the UI is always loaded in before proceeding. Note when you call :WaitForChild on a parent, all of it’s children will almost always be loaded in with the parent, which is why I only need to call it once on the ScreenGui. Also, since the functions connected to the events are doing the exact same thing (except for toggling true/false), you can put it all inside of one function and then call it with a value to pass to it, like this:

local player = game.Players.LocalPlayer
local playerGui = player:WaitForChild("PlayerGui")
local sGui = playerGui:WaitForChild("ScreenGui")
local close = sGui.X
local Black = sGui.TextButton
local Blue = sGui.TextButton2
local Orange = sGui.Textbutton3
local Open = sGui.Open

local function toggleVisible(status)
    Black.Visible = status
    Blue.Visible = status
    Orange.Visible = status
    Open.Visible = not status
    close.Visible = status
    print(status and "closed" or "opened")
end)

close.MouseButton1Click:Connect(function() --Close Button
	toggleVisible(false)
end)

Open.MouseButton1Click:Connect(function()-- Open Button
	toggleVisible(true)
end)

Sorry if this is written badly, I’m on mobile.

2 Likes
#5

To improve the script heres some notes

  • Use less variables
  • Use comments
  • Use this plugin

Thats it!
heres the example script.

   -- Variables --
   local text = "This is test"
   local character = game.ReplicatedStorage.Characters:GetChildren()

   -- Function --
   for i,v in pairs(character) do
     print(v.Name)
   end

  -- Other -- 
  print(text)

This is the example of well decorated script…

1 Like
#6

What? This has literally nothing to do with the original script he asked to improve

1 Like
#7

i told him example he can do like that…

1 Like
#8

Well that’s not even close to what he asked for, so…

1 Like
#9

read the title, he want to improve his script, and he can use them…

comments will help him to see what the function does… and it will be easy to find…

1 Like
#10

I really don’t see how adding comments can improve a script other than making it easier for someone to understand, but if you are the only person that will be seeing the script it wouldn’t matter too much in that case.

1 Like