Any feedback on the code and the module is welcomed!
Can you publish your source code to GitHub or other web based alternatives like Pastebin?
I looked at the code really quickly, and I found out that you are using while wait() do loop. May I ask why you are doing so?
And I pretty sure this can be done easily using a few lines of code, lets say we were to add a cooldown to button clicking:
local inCooldown = false
local button = path.to.button
button.MouseButton1Click:Connect(function()
if not inCooldown then
inCooldown = true
-- do stuff
-- after doing stuff
inCooldown = false
end
end)
And if you would like to use os.time() to do so:
local oldTime = 0
local cooldownTime = 5 -- 5 second cooldown
local button = path.to.button
button.MouseButton1Click:Connect(function()
if os.time() - oldTime > cooldownTime then
oldTime = os.time()
-- not in cooldown, do stuff
else
-- in cooldown
end
end)
No idea why you need a loop for that, correct me if I am wrong.
That’s a very good question and I will try to answer it as best as I could.
The part of the code you gave me is from UpdateTime function that is created as a new thread in WaitCooldown function that just waits the cooldown time (and at the end removes it from the table). What UpdateTime function does is it updates the value inside the table (the CooldownList table would look like this {“the2dguy” = {“button” = 5}}). That way as time progresses the value will change (in seconds) and I used the wait() loop as it would be more accurate since it has 0.03-second minimum delay. You can think of UpdateTime function as a timer that changes the cooldown duration based on how much time was passed. Once the “button” value reaches 0 the while loop will break (stop).
You might ask why update the time inside the table?
Well, for example, let’s say you have a feature that spawns a nuke in your game but you want the player to spawn a nuke only once every 5 minutes. In this case, the UpdateTime function becomes very useful as the “Check” function as you know returns 2 variables. The second variable is time that is left of the cooldown so then what you could do is tell the player how much time is left before he could set another nuke.
Here is an example:
Hopefully, that answered your question and the reason why I have two functions is that so it would be easier for you to remove “UpdateTime” function if you don’t really need it.
I would strongly advise that you look further into some of your code surrounding loops and your use of the wait()
function. As far as your loop goes, you are relying on wait()
to return a truthy value. That could be mitigated by removing the wait()
function entirely from your loop’s conditional statement. However, that is not your solution. You should use an event-driven approach using RunService events. I would also strongly advise that you look into Roblox’s new task library. That’s your solution.
You may find these links helpful:
Task Scheduler (Roblox)
RunService (Roblox)
Task Library (Roblox)
Spawn is evil (evaera)
I’ve read the links you’ve given me from the developer.roblox.com and this is what I have to say.
I’ve implemented the UpdateTime function in seconds (1, 2, 3) to show the person how much time he/she still has to wait until the cooldown is finished. I totally understand that the wait() function is slower than RunService events but I’m not trying to be so precise on the time frame as it is just in seconds. So, it won’t really matter if it would always update every frame as the timer is in seconds and for the person, this delay won’t mean anything.
If I’m missing some other benefit of using RunService please correct me.
Take a further look at the “Spawn is evil” link. The problem with it doesn’t have anything to do with precision. Using the wait
function is appropriate for loops that you want to happen over an extended interval (think like every 10 seconds or something).
If you’d like a more technical explanation, I’d be happy to explain it in its entirety. The post made by evaera (a recognized programmer who currently works for Adopt Me as a senior systems engineer) should give you a general idea though.
It stated it’s fine using wait() as a timer as I do in the WaitCooldown function.
The reason why I still would use the wait() function instead in a while loop is cause I do not need a fast resume and accurate refresh rate for a countdown function of the cooldown. The reason for it being is that a high amount of UpdateTime functions running could cause lag spikes. The importance of accuracy here is also not really needed, either 23 seconds left or 22 seconds for a human this won’t really matter.
Here is a quick video I made showcasing the lag spikes: - YouTube
To anybody who has been using the cooldown module I’ve changed the code line order a bit for the UpdateTime function (in case the table or the value were removed by WaitCooldown function). So, I recommending getting the cooldown module again from the toolbox.
The screenshot you provided clearly shows them using 30 seconds as an interval, unlike your 1/30th of a second interval. If I was going to use your code, I would fork it and make fixes for it. I simply don’t see how anyone could be okay with denounces potentially hanging for a random amount of time.
There are 2 instances where I used the wait() function one in WaitCooldown() and another for while loop. It’s not a random amount of time as shown in a video its average delay (with around 100 cooldowns running at the same time) would be around 0.16 seconds. Which would be a good refresh rate just for a countdown in seconds. This slower refresh rate allows more space for more calls causing no lag spikes in the video. If I would use RunService.Heartbeat or RunService.Stepped it would have a much faster refresh rate but would cause lag spikes on the server.
The cooldowns won’t hang for a random amount of time as they are removed after the wait(Cooldown time) function finishes no matter if the countdown finished or not.
If I would have a game with 20 players making 1-2 cooldown calls every 2-3 seconds I wouldn’t want it to lag the game.
Here’s yet another post explaining why you’re wrong.
I suggest you read them and do your research. Based on your replies, I can’t tell if you aren’t reading them or if you are misinterpreting them.
| Version 2.0 |
So, after long thinking, I’ve come to the solution of how to optimize the module. With the request of @ForcyDorcy to use the RunService event instead of the wait() function. I came with a way that wouldn’t lag the game, with 100 cooldown request calls (or more) using the RunService.
The solution was simple instead of creating 2 new coroutines (threads) for each cooldown; now it will create one wrap coroutine; for cooldown timer that does the same job as WaitCooldown and UpdateTime function for all of the cooldowns in the CooldownData (CooldwonList). With that amount of accuracy in hand, I decided on the Check function; to not only return time in seconds but also in milliseconds. If you want to round it to seconds: simply use math.floor(timeLeft).
The module was updated, so now it should be much accurate on timing.
Just so you’re aware, coroutines aren’t going to speed anything up. Also, coroutines aren’t threads. There is a substantial difference between the two.
The threads contained each a different for loop with RunService event, which does cause lag at huge quantity.
I encourage you to do more research on everything you’ve been posting. You’re doing a huge disservice to inexperienced developers when you post misleading information. I’ve tried correcting you several times, but you seem to ignore any corrections. I will consider forking your source code and correcting it on my own at a later date.
You have full permission to do so. I do know that coroutines dont speed anything but I had to create it in order to run a for loop in a new thread. I did change wait() function to RunService.Heartbeat:Wait() so I dont see what’s your problem now. Theres now much more less delay and the finish/resume time of the cooldown is now much more accurate.
And I did realize that I did lie about the lag that was caused, it was mostly because of the print() function that was in the loop (that was the main reason why I wanted to use wait() instead). However, I did optimize the module instead of creating 2 new threads for every cooldown and wait() function; now it would create only one thread for the timer that would go through all of the cooldowns that use’s RunService.Heartbeat:Wait().
And instead of telling me that I give misleading information and telling me that I’m wrong, you could’ve corrected me and explain why.
| Version 2.5 |
Added 2 more variables for the Add function. Now you can include the callback function as a fourth variable, which will run after the cooldown duration is finished. The last variable would be the argument(s) that are passed to the callback function. Both of the variables are not required and are optional.
Example of usage:
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local CooldownModule = require(ReplicatedStorage.Module)
function LaunchRocket(args)
print(args[1].." has launched a rocket.")
--Code
end
ReplicatedStorage.StartRocket.OnServerEvent(function(player)
local checkResult, timeLeft = CooldownMod.Check(player.Name, "StartRocket")
if checkResult then
CooldownMod.Add("AllAllLol1", "StarRocket", 10, LaunchRocket, {player.Name})
else
ReplicatedStorage.StartRocket:FireClient(player, checkResult, timeLeft) --return checkResult and time that is left until the finish of the cooldown
end
end)
Example of misusage:
CooldownMod.Add("AllAllLol1", "StarRocket", 0, LaunchRocket, {player.Name})
Keep in mind that every new callback function is ran in new corountine.wrap function and that the callback function is not recommended to be used instead of Thread:Delay() module function.
If you have any questions or problems with the module, let me know as I haven’t tested it much.
Thanks I will definitely use this for throwable cooldowns