Improve script and make it smaller

Help and Feedback Code Review
#1

Hey, I had a question. I have a pretty large script rn and I questioned myself is there a way too make it shorter and since Im still a complete beginner idk what the best way is to do that

wait(2)
local cs = game:GetService("SoundService").SFX.Click
local Operator = script.Parent.Operator.Value

local function Tweening(frame, location)
	frame:TweenPosition(UDim2.new(location), --position
		Enum.EasingDirection.Out, --Direction
		Enum.EasingStyle.Back, --Style
		1, --Time
		false) --again?
end

local HomeFrame = script.Parent.Parent.BaseFrameHome
local SettingsFrame = script.Parent.Parent.BaseFrameSettings
local DeployFrame = script.Parent.Parent.BaseFrameDeploy
local StoreFrame = script.Parent.Parent.BaseFrameStore
local CreditsFrame = script.Parent.Parent.BaseFrameCredits
local ServersFrame = script.Parent.Parent.BaseFrameServers
local OverheadFrame = script.Parent

local HomeButton = script.Parent.Home
local StoreButton = script.Parent.Store
local ServersButton = script.Parent.Servers
local SettingsButton = script.Parent.Settings
local CreditsButton = script.Parent.Credits
local DeployButton = script.Parent.Parent.BaseFrameHome.DeployFrame.Detector
local ReturnDeployToHome = DeployFrame.ReturnFrame.Detector

local SettingsButton2 = HomeFrame.SettingsFrame.Detector
local CreditsButton2 = HomeFrame.CreditsFrame.Detector
local StoreButton2 = HomeFrame.StoreFrame.Detector

local function ToHome()
	if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 0,0,0,0)
		Tweening(SettingsFrame, 2,0,0,0 )
		Tweening(CreditsFrame, 4,0,0,0)
		Tweening(StoreFrame, -2,0,0,0)
		Tweening(ServersFrame, -4,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end
local function ToSettings()
	if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, -2,0,0,0)
		Tweening(SettingsFrame, 0,0,0,0 )
		Tweening(CreditsFrame, 2,0,0,0)
		Tweening(StoreFrame, -4,0,0,0)
		Tweening(ServersFrame, -6,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end
local function ToCredits()
	if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, -4,0,0,0)
		Tweening(SettingsFrame, -2,0,0,0 )
		Tweening(CreditsFrame, 0,0,0,0)
		Tweening(StoreFrame, -6,0,0,0)
		Tweening(ServersFrame, -8,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end
local function ToStore()
	if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 2,0,0,0)
		Tweening(SettingsFrame, 4,0,0,0 )
		Tweening(CreditsFrame, 6,0,0,0)
		Tweening(StoreFrame, 0,0,0,0)
		Tweening(ServersFrame, -2,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end
local function ToServer()
	if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 4,0,0,0)
		Tweening(SettingsFrame, 6,0,0,0 )
		Tweening(CreditsFrame, 8,0,0,0)
		Tweening(StoreFrame, 2,0,0,0)
		Tweening(ServersFrame, 0,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end

local function ToDeploy()
	if Operator == true then
		print("in session")
	else
		Operator = true
		HomeFrame:TweenPosition(UDim2.new(0, 0,-2, 0), --position
			Enum.EasingDirection.Out, --Direction
			Enum.EasingStyle.Back, --Style
			1, --Time
			false) --again?

		OverheadFrame:TweenPosition(UDim2.new(0, 0,-1, 0), --position
			Enum.EasingDirection.Out, --Direction
			Enum.EasingStyle.Linear, --Style
			1, --Time
			false) --again?

		DeployFrame:TweenPosition(UDim2.new(0, 0,0, 0), --position
			Enum.EasingDirection.Out, --Direction
			Enum.EasingStyle.Back, --Style
			1, --Time
			false) --again?
		cs:Play()
		wait(1)
		Operator = false
	end
	
end
local function DeployToHome()
	if Operator == true then
		print("in session")
	else
		DeployFrame:TweenPosition(UDim2.new(0, 0,2, 0), --position
			Enum.EasingDirection.Out, --Direction
			Enum.EasingStyle.Back, --Style
			1, --Time
			false) --again?

		OverheadFrame:TweenPosition(UDim2.new(0, 0,0, 0), --position
			Enum.EasingDirection.Out, --Direction
			Enum.EasingStyle.Linear, --Style
			1, --Time
			false) --again?
		ToHome()
	end
end


ReturnDeployToHome.MouseButton1Up:Connect(function()
	DeployToHome()
end)
DeployButton.MouseButton1Up:Connect(function()
	ToDeploy()
end)

HomeButton.MouseButton1Up:Connect(function()
	ToHome()
end)
SettingsButton.MouseButton1Up:Connect(function()
	ToSettings()
end)
SettingsButton2.MouseButton1Up:Connect(function()
	ToSettings()
end)
CreditsButton.MouseButton1Up:Connect(function()
	ToCredits()
end)
CreditsButton2.MouseButton1Up:Connect(function()
	ToCredits()
end)
StoreButton.MouseButton1Up:Connect(function()
	ToStore()
end)
StoreButton2.MouseButton1Up:Connect(function()
	ToStore()
end)
ServersButton.MouseButton1Up:Connect(function()
	ToServer()
end)
4 Likes
#2

Ah yes, I love tweens. There are some things I can suggest.

You don’t have to do callback function when connecting the events. For example:

Before

HomeButton.MouseButton1Up:Connect(function()
	ToHome()
end)

After

HomeButton.MouseButton1Up:Connect(ToHome)

Doesn’t it look a lot nicer?

Or…you can do the table and loop method to clean up your code and make it not repetitive. For example

local button1Ups = {
    [HomeButton] = ToHome,
    [SettingsButton] = ToSettings,
    -- ...
}

for k, v in pairs(button1Ups) do
    k.MouseButton1Up:Connect(v)
end

Happy cleaning! I haven’t tested and debug it, so rather take this as a structure guide.

Also, access the user’s Gui by doing

local playerGui = game:GetService("Players").LocalPlayer:WaitForChild("PlayerGui")

Don’t make it hard on yourself by accessing through spamming Parents.

4 Likes
#3

Woah thx I love this Im still learning how to script so I didnt know a lot of this was possible

2 Likes
#4

btw im encountering 1 litlle problem with the second line of code bc Im not really handy using a table as a result of smt like I when I play the game it just runs through the the buttons I dont have to press them this is the what I changed it in it def looks a lot better but I just runs without me having to press the buttons

local button1Ups = {
[HomeButton] = ToHome(),
[SettingsButton] = ToSettings(),
[SettingsButton2] = ToSettings(),
[CreditsButton] = ToCredits(),
[CreditsButton2] = ToCredits(),
[StoreButton] = ToStore() }

for k, v in pairs(button1Ups) do
k.MouseButton1Up:Connect(v)
end

1 Like
#5

You cannot run the functions inside the table else it happens. You have to take away the parenthesis () from the functions in the table. They are meant to be accessed by events.

Properly working version

local button1Ups = {
  [HomeButton] = ToHome,
  [SettingsButton] = ToSettings,
  [SettingsButton2] = ToSettings,
  [CreditsButton] = ToCredits,
  [CreditsButton2] = ToCredits,
  [StoreButton] = ToStore
}

for k, v in pairs(button1Ups) do
  k.MouseButton1Up:Connect(v)
end
2 Likes
#6

woah your right!! Im gonna read a litlle more abt tables since Im happy it works but I got no clue why :sweat_smile:

3 Likes
#7

woah thx for the help you learned me how much value a table and for iv in pairs loop can add to a script

local button1Ups = {
[HomeButton] = ToHome,
[SettingsButton] = ToSettings,
[SettingsButton2] = ToSettings,
[CreditsButton] = ToCredits,
[CreditsButton2] = ToCredits,
[StoreButton] = ToStore,
[StoreButton2] = ToStore,
[ServersButton] = ToServer,
[DeployButton] = ToDeploy,
[ReturnDeployToHome] = DeployToHome
}

for k, v in pairs(button1Ups) do
k.MouseButton1Up:Connect(v)
end

its so short now :muscle:t3:

3 Likes
#9

Besides everything that has been said…

You don’t need to compare something to true because the == operator actually returns a boolean, like it’s a complete redundancy.
For instance, this code:

local var = true
if var == true then
  print("Is true")
end

Will do the exact same as this:

local var = true
if var then
  print("Is true")
end

And this:

local var = true
if var then print("Is true") end

So, every time you have something like this:

if Operator == true then
		print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 0,0,0,0)
		Tweening(SettingsFrame, 2,0,0,0 )
		Tweening(CreditsFrame, 4,0,0,0)
		Tweening(StoreFrame, -2,0,0,0)
		Tweening(ServersFrame, -4,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end

You may rewrite it to:

    if Operator then print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 0,0,0,0)
		Tweening(SettingsFrame, 2,0,0,0 )
		Tweening(CreditsFrame, 4,0,0,0)
		Tweening(StoreFrame, -2,0,0,0)
		Tweening(ServersFrame, -4,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end

Or just:

    if Operator then
       print("in session")
	else
		Operator = true
		Tweening(HomeFrame, 0,0,0,0)
		Tweening(SettingsFrame, 2,0,0,0 )
		Tweening(CreditsFrame, 4,0,0,0)
		Tweening(StoreFrame, -2,0,0,0)
		Tweening(ServersFrame, -4,0,0,0)
		cs:Play()
		wait(1)
		Operator = false
	end
end

Hope this helps!

1 Like
#10

@UnrealForUnreal yeah well I never really understood that bc that checks if the operator is equal to itself right? or does it check or the operator has the value I set for it at the start? thx btw for the second option I didnt know that was possible thx that makes a lot of my other script a lot shorter too

#11

Look, think it this way:
The == operator returns a Boolean that says if two values are equal.
So, what would be the value of this?:

1 == 1

It’s true!

Code exemplification

For instance, try running this code:

local var1 = (1 == 1) -- parenthesis are redundant, but they make it more readable
print(var1) -- prints true

local var2 = (2 == 1)
print(var2) -- prints false

So, let’s see what happens with the == true expression:

local var1 = (true == true) -- (true == true) = true
print(var1) -- prints true

local var2 = (false == true) -- (false == true) = false
print(var2) -- prints false

You see the pattern?: The original bool remains untouched when compared to true… so , comparing to true changes absolutely nothing and you may get rid of it.

Extra: Verbal exemplification

In other words, when comparing something to true you’re saying:
“This apple does exists. Does it? Yes, it does.” (that last part is completely redundant/dispensable), or:
“This pear does not exists. Does it? No, it does not.” (that last part is completely redundant/dispensable).
In both of them, that “Does it?” (== true) is completely redundant because it doesn’t change the sense of the sentence. The same thing happens in an if statement, it doesn’t change anything, so it must be avoided to reduce number of words, thus the performance of the program.

Hope this helps!

1 Like
#12

woah thx for the great explanation I tot get it now :muscle:t3:

1 Like
#13

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.