Disclaimer: The term “queuing” explained below is something I came up with. If there are any other forms of “queuing” in general coding or in the dev forum space, I am not referring to those as such in this topic.
Recently I have had a problem with finding out how to destroy a certain thread once a condition is met. The solution I have come up with is a “queuing” system where I can make the previous thread not be able to run as that thread is considered “dead” when a certain condition is met.
What I want to look for is a more optimized, cleaner code that is probably easier to read without the need of checking what value the queue value is currently at. Not only that, but I am also looking to see if this current code can be further optimized for better performance. In another word, is there a better way to optimize this code?
The code below demonstrates that if a condition were to change at any moment, the thread is regarded as “dead” and won’t be executed. The only way the code would execute is if the player waited the full “reload time” with their guns out.
-- module script
function gunMechanic.reload(localPlayer, weapon, reloadAnimation, reloadSound)
if BULLET_IN_CLIP.Value ~= AMMO_CAPACITY.Value and IS_RELOAD.Value == false then
reloadQueue += 1
IS_RELOAD.Value = true
local gunMechanic = require(ReplicationStorage.gunMechanic)
local cooldownTween = gunMechanic.cooldown(localPlayer, weapon, RELOAD_TIME.Value)
-- The code below cancels reloading animation and cooldown tween when weapon is unequipped or finish reloading
task.spawn(function()
while IS_RELOAD.Value == true and IS_EQUIPPED.Value == true do
task.wait()
end
IS_RELOAD.Value = false
cooldownTween:Cancel()
reloadAnimation:Stop()
end)
reloadAnimation:Play()
print("reloading...")
task.wait(RELOAD_TIME.Value)
-- The code below executes when the player successfully waits for the gun to reload without unequipping
if reloadQueue == 1 and IS_EQUIPPED.Value and IS_RELOAD.Value then
print("reloaded")
BULLET_IN_CLIP.Value = AMMO_CAPACITY.Value
IS_RELOAD.Value = false
gunMechanic.updateAmmo(localPlayer, weapon)
end
reloadQueue -= 1
end
end
This is a common problem. Indeed, you cannot kill a thread from the outside, but there are simple solutions to do so. The biggest problem with your current code is that you have two delayed threads running in parallel, one obviously to delay your task, and one to listen when your delay is being canceled. To formalize what you’re trying to achieve, I will call it a “timeout”, it’s just a function to be run after some time. You want to be able to cancel externally this timeout before it is executed, and also to run some function onCancel upon cancellation. The problem is originated from the fact that you want to listen for cancellation, and run onCancel, while you should just run onCancel when you are actually canceling, e.g. when your variables change.
Regarding onCancel, if it didn’t rely on the context from when you initialized the timeout, you could’ve just stored it anywhere and run it upon cancellation. However, yours relies on the initial context, so you’ll have to create it where you are creating your timeout, and then store it in a place where it’ll be accessible externally.
Now regarding the timeout itself, your implementation will also have a problem with concurrency. It’s probably why you added that variable reloadQueue, but actually the right way to think about it is to see each timeout as an instance that knows whether or not it should still be alive. Since you don’t want people to reload while they’re already reloading, it makes it easier. However, you should probably consider switching to OOP for your guns, allowing them to have a state. It would work like this:
function GunMechanic:reload(...)
if not self.reloading then
self.reloading = true
-- do stuff, start your tween and animation
local shouldContinue = true
self.cancelReload = function ()
shouldContinue = false
self.reloading = false
-- everything to be done on cancellation, e.g. stop your tween and animation
end
task.wait(duration)
if shouldContinue then -- hasn't been cancelled
-- the timeout's task, e.g. finalizing the reload
self.reloading = false
end
end
Then whenever your tool is unequipped, call self.cancelReload.
Ohh this makes sense and is very organized. I think I was overcomplicating myself with the queue system when I could have created a function inside the function with its own local variable. Just a quick question, I haven’t quite seen/I am new to this version of functions in a module script, so for self.cancelReload, is it a separate function I need to make inside the module script? If not, how do I call this function inside the unequip event? Thank you for your suggestions btw
I actually just figured task.cancel has rolled out into the engine very lately, so you could make the above code a bit simpler by storing the thread from task.delay but the thing is that you will still need to also store the onCancel function. This can be solved easily as I demonstrated previously with OOP, read more about this here: All about Object Oriented Programming
Note that using OOP for your guns will make the implementation a lot more natural and flexible. You will be able to store state variables such as current ammo in actual variables which are tens of times faster than Roblox Values. It will also allow you to store any type of data, not just the types available for Values.
OOP will definitely make my code flexible and reduce redundancy each time I need to call a certain function. I’ve been attempting to create a somewhat OOP-type structure in my module script to make it easier to create faster updates and also I am working with Java outside of Roblox. Just to be clear on the onCancel function, am I storing the thread when I call the reload function and running the onCancel through the stored value? Like in self.cancelReload, is that a separate function I need to implement?
function GunMechanic:reload(...)
if not self.reloading then
-- ...
self.cancelReload = function ()
shouldContinue = false
self.reloading = false
-- everything to be done on cancellation, e.g. stop your tween and animation
end
-- ...
end
end
Quoting a part of my previous code, and this part is definitely different from Java, you are assigning the cancelReload property of your object as a new function every time you start a new timeout. We are doing this to encapsulate the context within the function’s closure, so then you just have to call it. An alternate solution would be to store onto your object all the variables from the timeout creation that you need to access on cancellation, like the animation and tween, and just do stuff with these stored values on cancellation. Might quickly get more complicated to deal with, but it’s a good option if you don’t like the idea of storing a function temporarily.
