It should be if roundTime <= 590 and not teleportedPlayer then because you want to check if you have NOT teleported the player.
1 Like
It’s not just the UI being changed, there are several things it’s looking it over. As mentioned earlier, it takes way longer to countdown for some reason using a for loop
Ok so, first off change the while loop to
for i = length, 1, -1 do
end
And if player is Murderer, there is a wait so that will delay the thing. So you could use
spawn(function()
end)
so that the code apart from the wait does not affect the loop.
If it took much longer to countdown with a for loop then you must have been doing something wrong.
Inside of the for loop, it was checking several things, that might be the issue but I prefer the while loop more
This should work. I moved the timer check into the loop, and also found a potential bug with the timing.
I also added some comments to explain things
If what you wanted was for rounds to last 600 seconds, and for the murderer to spawn when there’s 590 seconds left, this should accomplish that.
function module.StartFFGRound(length, chosenMurderer, map) -- in seconds
local outcome
game.ReplicatedStorage.Events.IdentifyPlayer:FireAllClients()
game.ReplicatedStorage.Values.GameInProgress.Value = true
-- this should be a for loop, not a while loop
-- while and for loops aren't different in speed
for i = length, 0, -1 do
local innocents = {}
local murderer = {}
local murdererHere = false
if i == length - 10 then
print("It's time")
module.TeleportMurdererToMapFFG(chosenMurderer, map.PlayerSpawns:GetChildren())
end
for i, player in pairs(game.Players:GetPlayers()) do
if player:FindFirstChild("Innocent") then
table.insert(innocents, player)
elseif player:FindFirstChild("Murderer") then
-- potential bug? if a murderer gets found, the length of the
-- game will slightly increase because of wait(.3)
-- this is likely why you thought for loops were slower
wait(.3)
murdererHere = true
table.insert(murderer, player)
end
end
status.Value = toMS(i)
if not murdererHere then
outcome = "murderer-left"
break
end
if #innocents == 0 then
outcome = "murderer-victory"
break
end
if i == 0 then
outcome = "time-up"
break
end
wait(1)
end
if outcome == "murderer-victory" then
local winner
for i, v in pairs(game.Players:GetPlayers()) do
if v:FindFirstChild("Murderer") then
winner = v.Name
end
end
status.Value = winner.." is the winner!"
end
if outcome == "time-up" then
status.Value = "Times up! No one wins!"
end
if outcome == "no-one" then
status.Value = "No one was the last standing!"
end
wait(5)
end
Overall this code definitely could be rewritten to be more event-based and easier to read, but all I’ve done here is changed it to what it looked like you wanted to do.
You should look at @jrelvas’s solution if you want to rewrite the code to be cleaner, as they’re on the right track. My solution just fixes the problem without adjusting the code for readability.
1 Like
The idea of using a Heartbeat is to ditch the loop. Heartbeat is much more precise than a loop since it always fires each frame, unlike a loop where you have to yield it using an imprecise method like wait-
All of the code that needs to be run constantly needs to be there, since this is essentially your “new loop”.
You have 3 different variables here:
local roundTime = 600 --Controls how much time the round has left
local completedStates = {} --Stores all states that were completed
local states = {} --A table that contains the different states for your round
local roundConnection = nil --Keeps track of the current connection; YOU NEED TO KEEP TRACK OF IT TO STOP IT WHEN THE ROUND IS DONE
completedStates simply makes sure that you’re not calling the same states again.
Each key in states represents a different time period, and their value is a function to be called when the current time is equal or less to that period. Why do we have to check if it’s less than? It’s simple, since the time of each frame can vary, the time period will most likely not be exactly the same as the time period you have defined.
For what you’ve asked in the OP, states would look something like this:
states = {
[590] = function() print("I'm called when a player needs to be teleported!") end)
[0] = function() print("I'm called when the round ends!") end)
}
Now, the most important part is actually setting up the loop!
function module.StartFFGRound(length, chosenMurderer, map) -- in seconds
--Clean-up previous round data
roundTime = 600
completedState = {}
roundConnection = RunService.Heartbeat:Connect(function(dt)
roundTime -= dt
for timePeriod, functionToCall in pairs(states) do
if roundTime <= timePeriod and not completedStates[timePeriod] then
functionToCall() --Call the function needed (You can also parse any arguments if you need them in the function)
completedStates[timePeriod] = true --Mark this state as complete
end
end
end)
end
The loop code is surprisingly not that hard. It simply reduces the delta time from the round’s time each frame, and then for every existing state, check if the state’s time period is less than or equal to the current round’s time and if it hasn’t been called before. If the check passes, then call the associated function and set the state as completed, so it can’t be called again.
Finally, you have to do the cleanup, this can be easily made with the function you assign the number “0” to, since it’ll run when the round ends.
function()
roundConnection:Disconnect() --Stop the code you bound to heartbeat from executing, since the round is over.
roundConnection = nil --Let the connection be garbage collected by Lua (eliminated)
end
This simply stops the round code from running after it’s over.
1 Like
Using a Heartbeat loop is unnecessary here, why not use a Heartbeat wait? There’s no need to do the timer code every frame.
edit i’m not going to continue arguing about it here to avoid straying off-topic
This makes the code easier to read and to maintain, meaning it’s also easier to add new states, instead of needing to slap a wait for each single state of your round. Using a custom task scheduler has the same performance implications in this case, since the time still needs to be tracked somehow.
It’s just like adding an if statement for each similar condition. It’s not easy to read or maintainable. Why do that when you can simply have an “unchanging” main code that is easy to read and easy to add states to? This also has the advantage of being able to add and remove specific states at runtime, which could be useful for special rounds. You’re essentially just going back to the “waiting” problem, but with a more precise time measure.
TL;DR: Hardcoding your logic makes it more unreadable and harder to maintain the more conditions you want to add.
1 Like
Yes, this was the bug, I reduced it, might as well remove the wait.
Regarding @jrelvas I’ll be looking over it and see how I’ll implement it. Thank you both!
2 Likes
To increase the accuracy on waiting 10 seconds, you can do this instead:
local RunService = game:GetService("RunService")
local start = os.clock()
while os.clock() - start < 10 do
RunService.Heartbeat:Wait() -- super high accuracy, "os.clock() - start" is duration
end
To explain this code pattern, it’s essentially a timer(that I have been using for a module that I have been working on).
1 Like