How efficient is this loading screen?

Help and Feedback Code Review
#1

I’ve created a custom loading screen and would like to know what I could improve on. Please let me know!

game.ReplicatedFirst:RemoveDefaultLoadingScreen()
local TweenService = game:GetService("TweenService")
local ContentProvider = game:GetService("ContentProvider")
local PlayerGui = game.Players.LocalPlayer:WaitForChild("PlayerGui")
local LoadingGui = script:WaitForChild("LoadingGui")
local GuiElements = {LoadingGui.MainFrame, LoadingGui.MainFrame.Logo}
local DetailCounters = {LoadingGui.MainFrame.Counters.Detail1, LoadingGui.MainFrame.Counters.Detail2, LoadingGui.MainFrame.Counters.Detail3, LoadingGui.MainFrame.Counters.Detail4, LoadingGui.MainFrame.Counters.Detail5,}
local AssetLoad = {game.Workspace:GetDescendants(), PlayerGui:GetDescendants(), game.ReplicatedStorage:GetDescendants()}
local LoadingCompleted = false
LoadingGui.Parent = PlayerGui
game.StarterGui:SetCoreGuiEnabled(Enum.CoreGuiType.All, false)

wait(1)

for i, Content in pairs(AssetLoad) do
	for i = 1, #AssetLoad do
		local Asset = Content[i]
		ContentProvider:PreloadAsync({Asset})
		local Progress = i / #AssetLoad * 100

		local function Detail(Counter)
			TweenService:Create(Counter, TweenInfo.new(1, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false), {BackgroundColor3 = Color3.new(0, 0, 0)}):Play()
		end

		if Progress >= 20 then
			Detail(DetailCounters[1])
			if Progress >= 40 then
				Detail(DetailCounters[2])
				if Progress >= 60 then
					Detail(DetailCounters[3])
					if Progress >= 80 then
						Detail(DetailCounters[4])
						if Progress >= 100 then
							Detail(DetailCounters[5])
							wait(1)
							LoadingCompleted = true
						end
					end
				end
			end
		end
	end
end

if LoadingCompleted == true then
	for i, Counter in pairs(DetailCounters) do
		TweenService:Create(Counter, TweenInfo.new(1, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false), {BackgroundTransparency = 1}):Play()
	end
	TweenService:Create(GuiElements[2], TweenInfo.new(1, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false), {ImageTransparency = 1}):Play()
	local MainTween = TweenService:Create(GuiElements[1], TweenInfo.new(1, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false), {BackgroundColor3 = Color3.new(0.117647, 0.117647, 0.117647)})
	MainTween:Play()
	
	MainTween.Completed:Connect(function()
		wait(1)
		TweenService:Create(GuiElements[1], TweenInfo.new(1, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut, 0, false), {BackgroundTransparency = 1}):Play()
		game.StarterGui:SetCoreGuiEnabled(Enum.CoreGuiType.Chat, true)
		game.StarterGui:SetCoreGuiEnabled(Enum.CoreGuiType.Health, true)
	end)
	
	wait(3)
	LoadingGui:Destroy()
	script:Destroy()
end

Thanks,
Skylexion

3 Likes
#2

  • Why wait(1)?

  • You can replace if LoadingCompleted == true then with if (LoadingCompleted) then

  • Also, I heard that wait is unstable/bad under heavy load.

  • You can use ipairs for arrays, which is faster

3 Likes
#3

You’re leaving memory leaks by not disconnecting/destroying the tweens, there are too many if statements (cognitive complexity over 7), as mentioned above you should use i-pairs for arrays and avoid using wait, you should organize more your code.

This loading screen wouldn’t affect the game’s performance, but it can definitely be improved.

I’ll leave these resources here:

1 Like
#4

Preloading everything is the same as preloading nothing, so that’s already a big point of inefficiency. Keep in mind that Roblox loading screens should only strive to load what needs to be immediately seen by the player (e.g. textures in the loading screen and main menu images).

There are also two other gotchas about PreloadAsync. The first thing is that it already loads an instance and its descendants, so you never actually need to use GetDescendants. The second thing is that an engineer discouraged using PreloadAsync in this manner where you pass each asset into a single-entry table. Passing a complete table loads your assets in parallel which is faster.

Beyond that, there are just various general improvements and nitpicks to be made.

  • The top of your script is fairly messy. Consider organising your variables into certain categories such as services, other objects and tables.

  • Please be consistent with getting services! Use GetService for all cases of needing services rather than changing between that and dot syntax. Do remember that services can be renamed which will break that access and GetService is canonical for fetching services.

  • PlayerGui implicitly exists to LocalScripts. You do not need a WaitForChild on it and you shouldn’t have one there either. The same goes for any content in ReplicatedFirst not needing WaitForChild: by the time LocalScripts run, ReplicatedFirst objects will have been replicated.

  • MainTween might leak memory. You can destroy it from within the function. At the end, just add in a quick MainTween:Destroy(). As for the wait(1), I’d recommend instead using the delay parameter of TweenInfo for the tween created in MainTween’s Completed, then connecting to that new tween’s Completed event and setting the CoreGuis from there.

4 Likes
#5

Please remove this wait.
image

Wait should only be used when delaying something. If you really needed it, you could just specify RunService as game:GetService("RunService") and add the following:

RunService.RenderStepped:Wait()

Otherwise, great job on your loading screen!