How could I simplify this module?

I’ve attempted to replicate the Roblox StarterGui:SetCore(“SendNotification”, {}) as I wanted it to fit in with the game UI.

The problem is, every time a new notification is created, the game lags for about .5 - 1 second/s. This is more of a problem than normal as some sounds then stop playing too early as the client has not caught up by the time the server removes them. The freezing is also just annoying in general.

I think it likely comes down to the script being too complex, so is there anything I can do better, or anything I could change to simplify it a bit?

Any help would be appreciated, thanks!

-- Services
local Players = game:GetService("Players")

-- Objects
local Player = Players.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")

-- Interfaces
local NotificationsInterface = PlayerGui:WaitForChild("NotificationsInterface").Container

-- Local variables
local titles = { "Incoming notification!", "New notification!" }

-- Module variables
local NotificationModule = {}
NotificationModule.Queue = {}
NotificationModule.Destroying = {}

NotificationModule.Running = false
NotificationModule.Paused = false

NotificationModule.AllowedNotifications = 3
NotificationModule.Notifications = 0

NotificationModule.Size = (NotificationModule.AllowedNotifications / 10)
NotificationModule.Padding = (1.05 / NotificationModule.AllowedNotifications)

-- Local functions
local function GetSound(tone)
	-- Find a tone
	if (script.Sounds:FindFirstChild(tone)) then 
		-- Clone the sound
		local sound = script.Sounds:FindFirstChild(tone):Clone()
				
		-- Remove the sound once it is done playing
		local endedConnection; endedConnection = sound.Ended:Connect(function()
			sound:Destroy()
			endedConnection:Disconnect()
		end)
				
		-- Return the sound
		return sound, endedConnection
	end
	
	return false
end

local function GetCurrentNotifications()
	-- Get the current notifications
	local notifications = NotificationsInterface:GetChildren()
	local tab = {}
	
	for ind, child in pairs(notifications) do
		-- Check if the child is not a GuiObject
		if (child:IsA("GuiObject")) then
			-- Remove the child from the table
			table.insert(tab, child)
		end
	end
	
	-- Sort the table, largest LayoutOrder first
	table.sort(tab, function(a, b)
		return a.LayoutOrder > b.LayoutOrder
	end)

	-- Return the new table	
	return tab
end

-- Regular functions
function PushNextNotification()
	-- Check if the system is not currently paused
	if ((not NotificationModule.Paused) and (not NotificationModule.Running)) then
		-- Check if there are notifications to be pushed
		if (#NotificationModule.Queue > 0) then
			NotificationModule.Running = true
			
			local notifications = GetCurrentNotifications()
																																											
			-- Check if there are currently notifications displayed
			if (NotificationModule.Notifications > 0) then 
				for _, frame in pairs(notifications) do
					-- Initialize a variable for the new position
					local newPosition = UDim2.new(0, 0, frame.Position.Y.Scale - NotificationModule.Padding, 0)
					
					-- Tween the object up
					frame:TweenPosition(newPosition, Enum.EasingDirection.Out, Enum.EasingStyle.Quart, 1/3)
					wait(1/3)										
				end
			end
			
			-- Push the new notification
			local notification = table.remove(NotificationModule.Queue, 1)
			local frame = script.Notification:Clone()
			
			-- Get the sound for the tone
			local sound = GetSound(notification.Tone)
			sound.Parent = frame

			-- Get a random title and change the title in the frame
			local title = titles[math.random(1, #titles)]
			frame.Title.Text = title
									
			-- Change the body text and the indicator colour			
			frame.Body.Text = notification.Body
			frame.Type.ImageColor3 = sound.Color.Value
			
			-- Play the sound
			frame.Parent = NotificationsInterface
			
			frame:TweenPosition(UDim2.new(0, 0, 1, 0), Enum.EasingDirection.Out, Enum.EasingStyle.Quart, 1/3)
			wait(1/6)
			
			sound:Play()
			wait(1/6)
			
			-- Increment the notifications
			NotificationModule.Notifications += 1
								
			coroutine.wrap(function()			
				wait(notification.Time)
																																																																			
				if (frame and (not NotificationModule.Destroying[frame])) then
					NotificationModule.Destroying[frame] = true
					
					frame:TweenPosition(UDim2.new(1, 0, frame.Position.Y.Scale, 0), Enum.EasingDirection.Out, Enum.EasingStyle.Quart, 1/3, true)
					wait(1/3)
					
					NotificationModule.Destroying[frame] = nil					
					frame:Destroy()
					
					NotificationModule.Notifications -= 1

					if (NotificationModule.Paused) then
						if (NotificationModule.Notifications == 0) then
							NotificationModule.Paused = false
							if (#NotificationModule.Queue > 0) then
								PushNextNotification()
							end
						end
					end
				end
			end)()
						
			-- Check if the notifications has reached their limits		
			if (NotificationModule.Notifications >= NotificationModule.AllowedNotifications) then
				-- Pause the system
				NotificationModule.Paused = true
				NotificationModule.Running = false
			else
				if (#NotificationModule.Queue > 0) then
					NotificationModule.Running = false
					PushNextNotification()
				end
			end
			
			if (NotificationModule.Running) then
				NotificationModule.Running = false
			end
		end
	end
end

-- Module functions
function NotificationModule.QueueNotification(message, tone, showTime)
	local sound, endedConnection = GetSound(tone)
	
	-- Check if there is a sound for the tone
	if (sound) then
		-- Add the notification to the table
		table.insert(NotificationModule.Queue, { Body = message, Tone = tone, Time = showTime })
						
		-- Check if the notification queue isn't full
		if ((NotificationModule.Notifications < NotificationModule.AllowedNotifications) and (not NotificationModule.Paused) and (not NotificationModule.Running)) then
			PushNextNotification()
		end	
		
		endedConnection:Disconnect()
		sound:Destroy()	
	end
end

-- Set up module
return NotificationModule

I would probably attribute the lag spikes to either the for loop, or something to do with the coroutine. I’d recommend trying the module with a wait(0) inserted in the last line of the for loop (at the start of the getcurrentnotifications function)

wait(n) - if n is lower than 0.03, it defaults to 0.03.

yeah, the 0 isn’t necessary, I normally write wait() instead. This can just stop the script from halting the game and allows it to continue rendering frames while the loop completes. It may reduce the efficiency of the module, but I don’t believe that should be an issue for sending notifications.

First of all, replace the module variables with normal variables, you’re creating bunch of references and taking up memory by doing so. You generally don’t want to have anything inside a “module table” unless you want them to be public, which you don’t want in this scenario. Also, if you only have one public function, why return a table and not the function.

1 Like

No… don’t use wait() at all. There are tons of other alternatives, explore Promise, RunService, Signals, custom Wait (60 Hz pipeline - front of queue)

1 Like

Thanks, I’ll change over to run service as its the fastest alternative I can find.
The lag spikes have stopped, marking as solution.