[Semi-Help] Help on Scripting Efficiency

C6T9 · May 15, 2020, 12:35pm

//
Hey there, my name is Keegan, an aspiring programmer here on Roblox.
I know that this current script works, but I was wondering if there is a more efficient way to do it.
//

local PetsButton = script.Parent.ButtonFrame.PetsButton
local StoreButton = script.Parent.ButtonFrame.StoreButton
local PetsFrame = script.Parent.MainFrame.PetsFrame
local StoreFrame = script.Parent.MainFrame.StoreFrame
local PetsLine = script.Parent.MainFrame.PetsFrame.Text.ImageLabel
local StoreLine = script.Parent.MainFrame.StoreFrame.Text.ImageLabel
local PetsOpen = script.Parent.PetsValue
local StoreOpen = script.Parent.StoreValue
local PetsClose = script.Parent.MainFrame.PetsFrame.CloseButton.TextButton
local StoreClose = script.Parent.MainFrame.StoreFrame.CloseButton.TextButton
local Camera = game:GetService("Workspace").CurrentCamera
local Blur = Instance.new("BlurEffect", Camera)
local debounce = true

Blur.Size = 0

local function closeblur()
    if debounce == false then
	debounce = true
        for i = 1,10 do
            Blur.Size = Blur.Size - 1
            wait(.01)
        end
	
    end
end

	local function openblur()
   	 if debounce == true then
		debounce = false
       	 for i = 1,10 do
           	Blur.Size = Blur.Size + 1
			wait(.01)
		end
		
	end
end


PetsButton.MouseButton1Click:Connect(function()
	if PetsOpen.Value == false then
		StoreOpen.Value = false
		PetsOpen.Value = true
		PetsFrame:TweenPosition(UDim2.new(.6, 0, .5, 0), 'Out', 'Elastic', 1, true)
		StoreFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		PetsLine:TweenSize(UDim2.new(.5, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		StoreLine:TweenSize(UDim2.new(0, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		openblur()
else
	if PetsOpen.Value == true then
		
		PetsOpen.Value = false
		PetsFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		PetsLine:TweenSize(UDim2.new(0, 0, .01, 0), 'In', 'Quint', .1, true)
		closeblur()
		print("1")
		end
	end
end)

PetsClose.MouseButton1Click:Connect(function()
	if PetsOpen.Value == true then
		PetsOpen.Value = false
		PetsFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		PetsLine:TweenSize(UDim2.new(0, 0, .01, 0), 'In', 'Quint', .1, true)
		closeblur()
		print("2")
	end
end)

StoreButton.MouseButton1Click:Connect(function()
	if StoreOpen.Value == false then
		PetsOpen.Value = false
		StoreOpen.Value = true
		StoreFrame:TweenPosition(UDim2.new(.6, 0, .5, 0), 'Out', 'Elastic', 1, true)
		PetsFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		StoreLine:TweenSize(UDim2.new(.5, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		PetsLine:TweenSize(UDim2.new(0, 0, .01, 0), 'In', 'Quint', .1, true)
		openblur()
else
	if StoreOpen.Value == true then
		StoreOpen.Value = false
		StoreFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		StoreLine:TweenSize(UDim2.new(0, 0, .01, 0), 'In', 'Quint', .1, true)
		print("3")
		closeblur()
		end
	end
end)

StoreClose.MouseButton1Click:Connect(function()
	if StoreOpen.Value == true then
		StoreOpen.Value = false
		StoreFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		StoreLine:TweenSize(UDim2.new(0, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		closeblur()
		print("4")
	end
end)

//
Script explanation: The UI is to Tween from the bottom up when opened (on click), During the UI opening, a blur is to happen in the background to emphasize the UI. Once another UI opens, the one that was currently on the screen tweens to the bottom, and the new clicked one tweens to the same position as the previous one. The “x” in the top right of both UI’s close the currently open one.
//
In-game representation: https://gyazo.com/5c5fe9d9c55a7acf00cfdd9bd56434cc
//
I am making this thread to try and improve my skills in scripting to become a more efficient scripter. I thank you very much for taking the time to view this thread, have an amazing rest of your day!

(I know there are more efficient ways, but being a beginner in scripting, I want to hear your guys’ imputs.)
//

Keegan, C6T9

sjr04 · May 15, 2020, 12:36pm

If your code works and you just want to look for ways to improve, move to #help-and-feedback:code-review. Also please don’t spam header titles, they make the text big and illegible.

xZylter · May 15, 2020, 12:37pm

If the script works utilize #help-and-feedback:code-review instead.

C6T9 · May 15, 2020, 12:37pm

Apologize for the header titles, was doing “- -” and didn’t know it emphasizes.

imalex4 · May 19, 2020, 11:00pm

Looks fine to me, I wouldn’t be worried about efficiency too much in a simple script like this.

Though, note that wait() may have a limit, which would mean doing wait(.01) would actually wait .01666 seconds or something. You can look into this if you wish, I could be wrong.

ArtFoundation · May 22, 2020, 1:15am

Well you could instead improve on the readability since the indentation got weird in some parts

ImpactHills · May 22, 2020, 4:59pm

Your code looks fine but you could make it more readable. Here’s some of my suggestions.

You should keep your indentation consistent (some of your code starts on different line widths)
Your conditional statements could be shortened.
Example:
if PetsOpen.Value == false then
Would be the same as
if not PetsOpen.Value then

Same thing with
if PetsOpen.Value == true then
Would be the same as
if PetsOpen.Value then
You should add each concept in it’s own function and every function should do One Thing Very Well!

For example you could make a function and call it DisplayPetsUI() and that function would only do one thing. Display the pets user interface.

local function DisplayPetsUI()
    PetsOpen.Value = true
    PetsFrame:TweenPosition(UDim2.new(.6, 0, .5, 0), 'Out', 'Elastic', 1, true)
    PetsLine:TweenSize(UDim2.new(.5, 0, .01, 0), 'Out', 'Quint', 1.5, true)
end

Then you could make a function and call it HideStoreUI() and that function would only do one thing. Hide the store user interface.

local function HideStoreUI()
    StoreOpen.Value = false
    StoreFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
    StoreLine:TweenSize(UDim2.new(0, 0, .01, 0), 'Out', 'Quint', 1.5, true)
end

You would make Display And Hide functions for both but now your code would be more manageable because your functions would do one thing and would be named correctly making it more readable for you and or your team. Now instead of calling it like this:

PetsButton.MouseButton1Click:Connect(function()
	if PetsOpen.Value == false then
		StoreOpen.Value = false
		PetsOpen.Value = true
		PetsFrame:TweenPosition(UDim2.new(.6, 0, .5, 0), 'Out', 'Elastic', 1, true)
		StoreFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		PetsLine:TweenSize(UDim2.new(.5, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		StoreLine:TweenSize(UDim2.new(0, 0, .01, 0), 'Out', 'Quint', 1.5, true)
		openblur()
else
	if PetsOpen.Value == true then
		
		PetsOpen.Value = false
		PetsFrame:TweenPosition(UDim2.new(.6, 0, 1.5, 0), 'Out', 'Elastic', 1, true)
		PetsLine:TweenSize(UDim2.new(0, 0, .01, 0), 'In', 'Quint', .1, true)
		closeblur()
		print("1")
		end
	end
end)

It would look like this:

PetsButton.MouseButton1Click:Connect(function()
	if not PetsOpen.Value then
        DisplayPetsUI()
        HideStoreUI()
        openblur()
    elseif PetsOpen.Value then
        HidePetsUI()
        closeblur()
    end
end)

You should keep your functions naming conventions consistent as well. I noticed you named your functions openblur() and closeblur() which isn’t bad but you should use lower camel case naming convention so you can easily read with your eyes that it’s two different words. openBlur() and closeBlur() is more eye catching and easier to read.

Hopefully you learned something from this.