Messy main menu module

Help and Feedback Code Review
#1

Hi,

I need help organizing my main menu module, there has to be a way to minimize this:

function MenuScreen.enableAsync()
	local PlayerGui = Player:WaitForChild("PlayerGui")
	MenuScreen.ScreenGui.Parent = PlayerGui
	
	-- logo size animation
	local MenuMusic = SoundService.Master.Background.Menu
	
	local minSize = 0.9 
	local maxSize = 1.1
	local smoothness = 0.05
	
	MenuScreen._logoConnection = RunService.PreRender:Connect(function(deltaTimeRender: number) 
		local loudness = math.clamp(MenuMusic.PlaybackLoudness / 130, minSize, maxSize)
		Logo:TweenSize(UDim2.fromScale(loudness, loudness), Enum.EasingDirection.In, Enum.EasingStyle.Linear, smoothness, true)
	end)
	
	-- seamless pattern
	local x,y = Pattern.TileSize.X.Scale * 2, Pattern.TileSize.Y.Scale * 2
	local speed = 5 -- higher = slower
	
	TweenService:Create(
		Pattern,
		TweenInfo.new(speed,Enum.EasingStyle.Linear,Enum.EasingDirection.In,-1),
		{Position = UDim2.new(-2*x,0,-2*y,0)}
	):Play()
	
	-- hover & functionality
	local hover = SoundService.Master.Interface.Hover
	local click = SoundService.Master.Interface.Click
	
	local transition = require(script.Transition)
	local playSound = require(ReplicatedStorage.Source.Util.playSound)

	local tweenInfo = TweenInfo.new(
		0.1, 
		Enum.EasingStyle.Quad, 
		Enum.EasingDirection.Out 
	)
	
	-- Main Menu
	for _, frame in pairs(Background:GetChildren()) do
		if not frame:IsA("Frame") then
			continue
		end

		if not frame:FindFirstChildOfClass("TextButton") then
			continue
		end

		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton
		local originalSize = frame.Size
		
		textButton.MouseEnter:Connect(function(x: number, y: number)
			local tween = TweenService:Create(frame, tweenInfo, {Size = UDim2.new(0.345,0,0.17,0)})
			tween:Play()

			playSound(hover,textButton)
		end)

		textButton.MouseLeave:Connect(function(textButton: number, y: number)
			local tween = TweenService:Create(frame, tweenInfo, {Size = originalSize})
			tween:Play()
		end)
		
		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			if Debounce then
				return
			end
			
			local canvas = Background.Parent:FindFirstChild(textButton.Parent.Name)

			if not canvas then
				return
			end
			
			playSound(click,textButton)
			transition.InAndOut(1)
			
			task.spawn(function()
				Debounce = true
				
				task.wait(1)
				
				Background.GroupTransparency = 1
				canvas.GroupTransparency = 0
				
				task.wait(1)

				Debounce = false
			end)
		end)		
	end
	
	local Statistics = MenuScreen.ScreenGui.Statistics
	-- statistics
	for _, frame in pairs(Statistics:GetChildren()) do
		if not frame:IsA("Frame") then
			continue
		end
		
		if not frame:FindFirstChildOfClass("TextButton") then
			continue
		end
		
		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton
		
		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			if Debounce then
				return
			end
			
			playSound(click,textButton)
			
			if textButton.Name == "Close" then
				return
			end
			
			transition.InAndOut(1)

			task.spawn(function()
				Debounce = true

				task.wait(1)

				Background.GroupTransparency = 0
				Statistics.GroupTransparency = 1

				task.wait(1)

				Debounce = false
			end)
		end)		
	end
	
	local Create = MenuScreen.ScreenGui.Create

	for _, frame in pairs(Create:GetChildren()) do
		if not frame:IsA("Frame") then
			continue
		end

		if not frame:FindFirstChildOfClass("TextButton") then
			continue
		end

		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton

		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			if Debounce then
				return
			end

			playSound(click,textButton)

			if textButton.Name == "Close" then
				return
			end

			transition.InAndOut(1)

			task.spawn(function()
				Debounce = true

				task.wait(1)

				Background.GroupTransparency = 0
				Create.GroupTransparency = 1

				task.wait(1)

				Debounce = false
			end)
		end)		
	end
	
	local Join = MenuScreen.ScreenGui.Join

	for _, frame in pairs(Join:GetChildren()) do
		if not frame:IsA("Frame") then
			continue
		end

		if not frame:FindFirstChildOfClass("TextButton") then
			continue
		end

		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton

		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			if Debounce then
				return
			end

			playSound(click,textButton)

			if textButton.Name == "Close" then
				return
			end

			transition.InAndOut(1)

			task.spawn(function()
				Debounce = true

				task.wait(1)

				Background.GroupTransparency = 0
				Join.GroupTransparency = 1

				task.wait(1)

				Debounce = false
			end)
		end)		
	end
	
	-- adjust strokes depending on viewport size
	UIStrokeAdjuster:TagScreenGui(Background.Parent)
end
#2

Hello, so you have a lot of code that’s just the same thing again, use functions for those.
I shortened your code by using functions, but you should probably look into CollectionService to handle these buttons using tags instead of looping through each frame.

I also moved some stuff outside of the function, because I think they shouldn’t be reevaluated every time the function runs, but I don’t have all of the context so I might be wrong.

local SoundService = game:GetService "SoundService"
local ReplicatedStorage = game:GetService "ReplicatedStorage"
local TweenService = game:GetService "TweenService"
local RunService = game:GetService "RunService"

local Player = game.Players.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")
MenuScreen.ScreenGui.Parent = PlayerGui--why isn't this done in studio?

local MenuMusic = SoundService.Master.Background.Menu
local hover = SoundService.Master.Interface.Hover
local click = SoundService.Master.Interface.Click

local transition = require(script.Transition)
local playSound = require(ReplicatedStorage.Source.Util.playSound)

local function onButtonActivated(textButton: TextButton)
	if Debounce then return end

	local canvas = Background.Parent:FindFirstChild(textButton.Parent.Name)
	if not canvas then return end 

	playSound(click,textButton)
	transition.InAndOut(1)
--spawning isn't necessary.
	Debounce = true
	task.wait(1)

	Background.GroupTransparency = 1
	canvas.GroupTransparency = 0

	task.wait(1)
	Debounce = false
end

local function addButtonf(parent: Frame)
	for _, frame in pairs(parent:GetChildren()) do
		if not frame:IsA("Frame") then continue end
		if not frame:FindFirstChildOfClass("TextButton") then continue end
		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton

		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			onButtonActivated(textButton)
		end)		
	end
end

--this function doesn't yield, so it shouldn't have the 'async' postfix.
function MenuScreen.enable()
	--I don't think there's a reason for the prerender connection, this just wastes memory
    --You can loop the tween instead. also :TweenSize is deprecated.
	local loudness = math.clamp(MenuMusic.PlaybackLoudness / 130, 0.9, 1.1)
	TweenService:Create(Logo, TweenInfo.new(0.05, Enum.EasingStyle.Linear, Enum.EasingDirection.In, -1), {Size = UDim2.fromScale(loudness, loudness)}):Play()

	local x,y = Pattern.TileSize.X.Scale * 2, Pattern.TileSize.Y.Scale * 2
	TweenService:Create(Pattern, TweenInfo.new(5,Enum.EasingStyle.Linear,Enum.EasingDirection.In,-1), {Position = UDim2.new(-2*x,0,-2*y,0)}):Play()
	
	local tweenInfo = TweenInfo.new(0.1, Enum.EasingStyle.Quad, Enum.EasingDirection.Out)
	for _, frame in pairs(Background:GetChildren()) do
		if not frame:IsA("Frame") then continue end
		if not frame:FindFirstChildOfClass("TextButton") then continue end

		local textButton =  frame:FindFirstChildOfClass("TextButton") :: TextButton
		local originalSize = frame.Size

--instead of original size, you could add some offset value in mouseenter, and subtract it again in mouseleave.
		textButton.MouseEnter:Connect(function(x: number, y: number)
			TweenService:Create(frame, tweenInfo, {Size = UDim2.new(0.345,0,0.17,0)}):Play()
			playSound(hover,textButton)
		end)

		textButton.MouseLeave:Connect(function(textButton: number, y: number)
			TweenService:Create(frame, tweenInfo, {Size = originalSize}):Play()
		end)

		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			onButtonActivated(textButton)
		end)		
	end

	addButtonf(MenuScreen.ScreenGui.Statistics)
	addButtonf(MenuScreen.ScreenGui.Create)
	addButtonf(MenuScreen.ScreenGui.Join)
	
	UIStrokeAdjuster:TagScreenGui(Background.Parent)
end

I added some suggestions in comments.
What do you think?

2 Likes
#3

Wow this really shortened my module by a lot and I can learn from this. Thank you very much. One issue was that when u removed the prerender connection, the logo didn’t change according to the loudness but it stayed the same meaning that it had to be updated using runservice. I’m also not as worried about memory because this game goes from menu → teleports you to actual game later, where then I actually have to worry about memory.

I have CharacterAutoLoads disabled so I have to do this

Thank you, it works perfectly.

New Code 
local CollectionService = game:GetService("CollectionService")
local ReplicatedFirst = game:GetService("ReplicatedFirst")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RunService = game:GetService("RunService")
local SoundService = game:GetService("SoundService")
local TweenService = game:GetService("TweenService")

local Assets = ReplicatedFirst.Assets

local transition = require(script.Transition)
local playSound = require(ReplicatedStorage.Source.Util.playSound)
local UIStrokeAdjuster = require(script.UIStrokeAdjuster)

local MenuScreen = {}
MenuScreen.ScreenGui = Assets.Menu:Clone() :: ScreenGui
MenuScreen._logoConnection = nil :: RBXScriptConnection?

local Player = game.Players.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")
MenuScreen.ScreenGui.Parent = PlayerGui

local Menu = MenuScreen.ScreenGui.Menu :: CanvasGroup
local Logo = Menu.Logo :: ImageLabel
local Pattern = Menu.Pattern :: ImageLabel

local MenuMusic = SoundService.Master.Background.Menu
local hover = SoundService.Master.Interface.Hover
local click = SoundService.Master.Interface.Click

local Debounce = false

local function onButtonActivated(textButton: TextButton)
	if Debounce then return end

	local canvas = Menu.Parent:FindFirstChild(textButton.Parent.Name)
	if not canvas then return end 

	playSound(click,textButton)
	transition.InAndOut(1)
	--spawning isn't necessary.
	Debounce = true
	task.wait(1)

	Menu.GroupTransparency = 1
	canvas.GroupTransparency = 0

	task.wait(1)
	Debounce = false
end

local function addButtonf(parent: Frame)
	for _, textButton in pairs(CollectionService:GetTagged("Button")) do
		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			onButtonActivated(textButton)
		end)		
	end
end

function MenuScreen.enable()
	MenuScreen._logoConnection = RunService.PreRender:Connect(function(deltaTimeRender: number) 
		local loudness = math.clamp(MenuMusic.PlaybackLoudness / 130, 0.9, 1.1)
		Logo:TweenSize(UDim2.fromScale(loudness, loudness), Enum.EasingDirection.In, Enum.EasingStyle.Linear, 0.05, true)
	end)
	
	local x,y = Pattern.TileSize.X.Scale * 2, Pattern.TileSize.Y.Scale * 2
	TweenService:Create(Pattern, TweenInfo.new(5,Enum.EasingStyle.Linear,Enum.EasingDirection.In,-1), {Position = UDim2.new(-2*x,0,-2*y,0)}):Play()

	local tweenInfo = TweenInfo.new(0.1, Enum.EasingStyle.Quad, Enum.EasingDirection.Out)
	
	for _, textButton: TextButton in pairs(CollectionService:GetTagged("Button")) do
		local frame = textButton.Parent
		local originalSize = frame.Size
		
		textButton.MouseEnter:Connect(function(x: number, y: number)
			TweenService:Create(frame, tweenInfo, {Size = UDim2.new(0.345,0,0.17,0)}):Play()
			playSound(hover,textButton)
		end)

		textButton.MouseLeave:Connect(function(textButton: number, y: number)
			TweenService:Create(frame, tweenInfo, {Size = originalSize}):Play()
		end)

		textButton.Activated:Connect(function(inputObject: InputObject, clickCount: number) 
			onButtonActivated(textButton)
		end)		
	end

	addButtonf(MenuScreen.ScreenGui.Statistics)
	addButtonf(MenuScreen.ScreenGui.Create)
	addButtonf(MenuScreen.ScreenGui.Join)

	UIStrokeAdjuster:TagScreenGui(Menu.Parent)
end

return MenuScreen
1 Like