Is this a good practice?

[StarJ3M]

So I made bombs in my game, and the way they work is they have a server script pick up the request to activate the bomb, and afterward, the server script tells a module script to perform all of the tasks to make the bomb function. The system is stable, but I wrote the script a while ago and recently noticed that I wrapped the code for performing the bomb’s functions inside of a task.spawn function.

I did it so that after using the module’s Explode() function, the cooldown would continue to run even after the code for the bomb ran since without it the cooldown would only start after the bomb’s code ran, but I’m wondering if this is a good practice and also if there is a better way to do this.

Server Script:

		local newBomb = bombModule.createBomb()
		newBomb.bombType = "Default"

		task.spawn(function()
			newBomb:Explode(user, bombData, bombClone, false, true)
		end)

		task.wait(bombData.cooldown)
		
		cooldown = false

Module Script:

bomb.Default = function(user, bombData, bombClone, plant, debugMode)
		local sound = Instance.new("Sound")
		sound.SoundId = bombData.tickSound
		sound.Parent = bombClone

		for i = 1, 3 do task.wait(bombData.tickTime) -- The timer on the bomb for explosion is ticking
			sound:Play()
			if i == 3 then task.wait(1) end -- prevents the bomb from exploding instantly on the third i = (tickTime * 3) + 1
		end

		if plant == false then bombClone.Anchored = true end -- We anchor the bomb so that it doesn't roll awaybomb.Anchored = true -- We anchor the bomb so that it doesn't roll awaw

		bombClone:SetAttribute("Exploded", true)

		local explosionSoundClone = Instance.new("Sound")
		explosionSoundClone.SoundId = bombData.explosionSoundId
		
		explosionSoundClone.Name = "explosionSound"
		explosionSoundClone.Parent = bombClone
		
		explosionSoundClone:Play()

		local explosionClone = explosionParticles(bombClone, bombData)
		explosionClone:Emit(100)

		bombClone.Transparency = 1

		local region3 = Region3.new( -- Creates a region3 that destroys any parts in the explosions radius.
			bombClone.Position - Vector3.new(bombData.range.L / 2, bombData.range.H / 2, bombData.range.W / 2),
			bombClone.Position + Vector3.new(bombData.range.L / 2, bombData.range.H / 2, bombData.range.W / 2)
		) -- Divide the range values by two to compensate for the size created by the region3 which is multiplied by two (e.x 16st = 32st)

		local explosionVisualizer
		if debugMode then explosionVisualizer = visualizerCalculator(region3) end

		local overlapParams = OverlapParams.new()
		if debugMode then overlapParams.FilterDescendantsInstances = {explosionVisualizer} end

		local parts = workspace:GetPartBoundsInBox(region3.CFrame, region3.Size, overlapParams)
		
		local appliedForcesOn = {} -- A table containing a list of objects that the force has already been applied on, preventing spam.
		for i, v in pairs(parts) do
			if v.Parent:FindFirstChildOfClass("Humanoid") then -- Blast away character models.
				local char = v.Parent
				local rootPart = char:FindFirstChild("HumanoidRootPart")  
				local humanoid = v.Parent:FindFirstChildOfClass("Humanoid")

				blastForce(rootPart, bombData, bombClone)
				
				
				if appliedForcesOn[char] == nil and (game.Players:GetPlayerFromCharacter(char)) == nil then -- For NPCs
				appliedForcesOn[char] = char

				humanoid:TakeDamage(bombData.damage)
				elseif appliedForcesOn[char] == nil and game.Players:GetPlayerFromCharacter(char).Team ~= user.Team then -- For Players
					appliedForcesOn[char] = char

					humanoid:TakeDamage(bombData.damage)
				end
				
			end
			if v.Name == "Brick" then -- For Bricks
				for i, constraint in pairs(v:GetChildren()) do
					if constraint:IsA("Constraint") then constraint:Destroy() end
				end

				v.Anchored = false
				v.CanTouch = false

				v.BrickColor = user.TeamColor

				if not table.find(appliedForcesOn, v) then
					local direction = (bombClone.Position - v.Position).Unit
					v.AssemblyLinearVelocity = Vector3.new(-(direction.X * bombData.blastPower)/5, bombData.blastPower/5, -(direction.Z * bombData.blastPower)/5)
					appliedForcesOn[v] = v -- Since for some reason most buildings get flung so far when the blast power is over 200, I decided to divide it by 5
				end -- to minimize the effect of the buildings getting flung.

				deleteTaggedBricks(v)
			elseif v.Name == "GameBrick" then v.Name = "TaggedGameBrick" -- TaggedGameBricks are GameBricks in the game that have been effected by the bomb, 
				-- while regular Bricks are just bricks you can destroy by default.
				for i, constraint in pairs(v:GetChildren()) do
					if constraint:IsA("Constraint") then constraint:Destroy() end
				end

				v.Anchored = false
				v.CanTouch = false

				v.BrickColor = user.TeamColor

				if not table.find(appliedForcesOn, v) then
					local direction = (bombClone.Position - v.Position).Unit
					v.AssemblyLinearVelocity = Vector3.new(-(direction.X * bombData.blastPower), bombData.blastPower, -(direction.Z * bombData.blastPower))
					appliedForcesOn[v] = v
				end

				deleteTaggedBricks(v)
			end
		end

		debris:AddItem(bombClone, 5)
		debris:AddItem(explosionVisualizer, 5)

		explosionVisualizer = nil
		table.clear(appliedForcesOn)
end

[Crygen54 (Crygen)]

You should post that in #help-and-feedback:code-review, and make sure to share the code with it so people can better help you.

To answer your question, it seem to be fine if you have to handle multiple bombs at same time which also have a cooldown that could yield the code and other bombs with it.

That’s basically one of the main use cases of creating new threads… As long as the bombs functions are optimized and the script activity don’t go skyrocket, it should be okay.