How could I improvise my explosion?

Help and Feedback Code Review
#1

My explosions system use a distance check to deal damage depending on distance but it’s not very efficient, how can I redo it?

local Explosion = game:GetService("ServerStorage").Core.Storage.Effects.RPGEX1:Clone()
Explosion.Anchored = true
Explosion.Position = Handle.Position
Explosion.Parent = workspace.Spawned
game:GetService("Debris"):AddItem(Explosion, 4)
for _,v in game:GetService("Players"):GetPlayers() do
	if v.Character then
		local Character = v.Character
		local PRT = Character.PrimaryPart.Position
		if (PRT - Explosion.Position).Magnitude <= 3 and (PRT - Explosion.Position).Magnitude >= 6 then
			Character:FindFirstChildOfClass("Humanoid"):TakeDamage(100)
		end
		if (PRT - Explosion.Position).Magnitude <= 6 and (PRT - Explosion.Position).Magnitude >= 12 then
			Character:FindFirstChildOfClass("Humanoid"):TakeDamage(50)
		end
	end
end
#2

Something like this should do.

-- Localize services once at the top.
local PlayerService = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

-- I assume the following part of the code runs when server receives a signal.
local Explosion = ServerStorage.Core.Storage.Effects.RPGEX1:Clone()
Explosion.Anchored = true
Explosion.Position = Handle.Position
Explosion.Parent = workspace.Spawned

-- task instead of Debris.
task.delay(4, function()
	for _, player in ipairs(PlayerService:GetPlayers()) do
		if not player.Character or not player.Character.PrimaryPart then
			continue;
		end
		
		local humanoid = player.Character:FindFirstChild("Humanoid")
		local rootPos = player.Character.PrimaryPart.Position
		
		-- Store magnitude in a variable once
		local magnitude = (rootPos - Explosion.Position).Magnitude
		
		if magnitude <= 6 then
			humanoid:TakeDamage(100)
		elseif magnitude <= 12 then
			humanoid:TakeDamage(50)
		end
	end

	Explosion:Destroy()
end)

Performance vice, the magnitude checks are definitely on the efficient side of the spectrum, compared to other methods I can think of. Following the DRY principle (“don’t repeat yourself”) and avoiding Debris service, the code snippet seems fine.

Why not use the Debris service? It was very commonly used in the past, however, today it’s better to use the more consistent task methods. Debris implements old 30 Hz wait(), which makes it no different than old spawn() and delay() in terms of execution priority and reliability.

I hope you don’t mind me changing the if-else statements, the values seemed to contradict, so I just left some example.

Edit: Spelling and forgot to fully replace Debris by destroying the explosion at the end.

#3

Good idea! But Should I use table functions for readibility?

local distanceDamages = {
	[6] = function(Humanoid)
		Humanoid:TakeDamage(100)
	end,
	[12] = function(Humanoid)
		Humanoid:TakeDamage(50)
	end,
}
distanceDamages[magnitude](humanoid)
#4

I’m very much in favor of organizing in tables, though in this case of a couple if-statements, some serious readability improvement doesn’t really seem necessary. It would maybe come useful if there were different effects, and so on, depending on the distance.

1 Like
#5

remove the for loop outside of task.delay cuz you are delaying the check aswell

#6

Why not use explosion.hit? You can exclude a model once it has already been hit, that way each player receives damage once rather than for each part in their character model.

#7

I use custom explosion particles. something Explosion doesn’t support.

#8

I see. I would still use an explosion instance for the simplicity of it. Set visibility to false and the joint-break percentage to 0. That way you can still overlay your explosion particles on top.

#9

What are you saying is I should create an explosion inside the particle part and do the fun on explosion instance instead?

#10

It shouldn’t matter where the explosion is placed, as it gets deleted instantly after. You would create the new instance and connect a hit function to it.

local HitPlayers = {}

local Explosion = Instance.new("Explosion")
Explosion.Parent = Workspace
Explosion.Position = **Position**
Explosion.Visible = false
Explosion.DestroyJointRadiusPercent = 0

Explosion.Hit:Connect(function(part)
     if part.Parent:FindFirstChild("Humanoid") then
          if HitPlayers[part.Parent] == false then
           **Deal Damage**
          HitPlayers[part.Parent] = true
          end
     end
end)

Something similar to this would do. The connection should disconnect when the explosion is deleted automatically.

1 Like