I just want to stick it in there that DataModel.(Is)Loaded is probably not an accurate way to handle a loading screen. Loading is only used to determine when the server finished replicating instances to the client, it doesn’t actually do any loading of any kind. Kind of irrelevant though.
Instead of using an upvalue bool as a condition, you should make use of the conditional statement where IsLoaded actually belongs. You’re reinventing the wheel by having two loops here.
Based on the code you’ve already provided, here’s a rewrite (not really spoon feeding code because you already have some, but I still would’ve liked to avoid this lol):
-- Typically good to index services first.
local Players = game:GetService("Players")
local ReplicatedFirst = game:GetService("ReplicatedFirst")
local TweenService = game:GetService("TweenService") -- PLEASE use this over the ugly GuiObject tween methods!
-- You'll want your objects next.
local LocalPlayer = Players.LocalPlayer
local PlayerGui = LocalPlayer:WaitForChild("PlayerGui")
local Gui = script.LoadingScreen -- ReplicatedFirst only runs once per client. Cloning is unnecessary.
-- Get to coding next.
PlayerGui:SetTopbarTransparency(0)
Gui.Parent = PlayerGui -- This is fine to do over cloning.
-- Don't add a wait after. Create your loop next.
-- Remember; MAKE USE OF THE CONDITIONAL STATEMENT.
do -- Open up a new scope so we can have a few local variables
local dots = 0 -- For later
local displayText = Gui.Frame.LoadingText
-- You were previously editing the wrong Gui's text, to note
while not game:IsLoaded() do
-- This is an interesting way to do it, and possibly better
displayText.Text = "Loading" .. string.rep(".", dots % 4)
wait(1)
end -- If the loop's done, it'll terminate here
print("Ready!") -- If you're up for debugging
displayText.Text = "Welcome to [REDACTED]!"
end -- Clean up scope and local variables we don't need
script.victory:Play() -- You should pick a better place to put this, like the Gui
-- Gui.victory:Play()
wait(5)
do -- Open up a new scope to stick some variables in
local TweenData = TweenInfo.new(0.5, Enum.EasingStyle.Sine, Enum.EasingDirection.InOut)
local Tween = TweenService:Create(Gui.Frame, TweenData, {Position = UDim2.new(0, 0, 1, 0)})
Tween:Play()
Tween.Completed:Connect(function ()
Gui:Destroy()
end)
end -- Close scope and clean up the variables
-- Code finished!
A little explanation to my code, if you’re curious some:
Why services first?
Later in your code, it can get very tedious to keep on writing out “game:GetService” if you want to access something. This is also a canonical way of fetching services - don’t use dot syntax, since the names of services can change and some of them aren’t even named properly in the DataModel. Services should go first since you may want to use them again later.
Why did you remove the Gui clone?
LocalScripts in ReplicatedFirst are run only once per client and are the first items to be replicated to the client. You can safely parent things out of ReplicatedFirst and it will still work as intended for every other client. Since the Gui exists in a script in ReplicatedFirst, it will also be replicated along with the code. A clone means you’re asking for another copy of the Gui to be made and replicated to the client. You can use the copy that’s already replicated instead and avoid reinventing the wheel unnecessarily here.
Why is your code in “do-end” blocks?
A do-end block:
do
-- Your code
end
These blocks are nice for executing “throw-away” code, I guess. Think of your entire script being in a do-end block, except you don’t actually write it out. These blocks are created instead so you can store a few local variables that you don’t need anywhere else in your script, it organises your code and nothing runs after it until the code in the do-end block has finished running. It’s pretty beneficial.
Why did you remove the ready value and replace it with IsLoaded?
Like I said, you’re reinventing the wheel by using two loops. The conditional statement is already available for you to use in a while loop, so make good use of it. By putting this here, I’m telling the loop to run until IsLoaded returns something that isn’t false. This is a better alternative and doesn’t require anything like additional threading via coroutines or spawn.
What is the string you wrote in the while loop?
Your original code would always wait 3 seconds explicitly; for every second, a dot would be added. My code truncates that to only add dots every time the loop runs around and the script can terminate at any time, not explicitly only after three dots have been added and IsLoaded doesn’t return false.
Why TweenService?
I don’t feel like explaining, too lazy. Just know that it can do everything that TweenSize/TweenPosition/TweenSizeAndPosition can do and more. It’s also far better.
