Simplifying and improving this RPG explosion

Help and Feedback Code Review
#1

I’m trying to simply this RPG explode thread to be as perfect, it works kind off but the distance checking can be off sometimes and checkin if player is npc or not can be tiring.

coroutine.wrap(function()
		Rocket.Touched:Connect(function(Hit)
			print("touched")
			if CanTouch and not Hit:IsDescendantOf(Tool.Parent) then -- Making sure rocket didn't hit the launcher.
				Touched = true
				CanTouch = false
				Rocket.Anchored = true
				Rocket.Transparency = 1
				Rocket:ClearAllChildren()
				for _,v in workspace:GetChildren() do
					if not v:IsA("Model") then continue end
					if v:IsA("Model") and v:FindFirstChild("Humanoid") and (v.PrimaryPart.Position - Rocket.Position).Magnitude <= Config.MaximumDistance then
						if game:GetService("Players"):GetPlayerFromCharacter(v) then -- Is player an npc?
							if RisytalModules:ReturnDistance(v, Rocket) <= Config.MaximumDistance then
								local Magnitude = FindInterval(RisytalModules:ReturnDistance(v, Rocket), Config) -- Get dictionary that's closest to player.
								v.Humanoid:TakeDamage(Magnitude.Damage)
								if Magnitude.Ragdoll then
									RisytalModules:ReturnF(v, "ev").Ragdoll:Fire(2.5)
									FlingP(Rocket, v.PrimaryPart, Magnitude.Force, Magnitude.StunTime)
								end
								if Magnitude.BrutalDeath == true then
									task.spawn(function()
										for _,p in v.ValuesFolder.LimbHealths:GetChildren() do
											if tostring(p):match("Head") then
												continue
											end
											p.Value = 0
										end
									end)
								end
							end
						else
							if v:GetAttribute("IsDummy") then
								if RisytalModules:ReturnDistance(v, Rocket) <= Config.MaximumDistance then
									local Magnitude = FindInterval(RisytalModules:ReturnDistance(v, Rocket), Config)
									v.Humanoid:TakeDamage(Magnitude.Damage)
									if Magnitude.Ragdoll then
										RisytalModules:ReturnF(v, "ev").Ragdoll:Fire(2.5)
										FlingP(Rocket, v.PrimaryPart, Magnitude.Force, Magnitude.StunTime)
									end
									if Magnitude.BrutalDeath == true then
										task.spawn(function()
											for _,p in v.ValuesFolder.LimbHealths:GetChildren() do
												if tostring(p):match("Head") then
													continue
												end
												p.Value = 0
											end
										end)
									end
								end
							end
						end
					end
				end
				Rocket:Destroy()
				Rocket = nil
			end
		end)
	end)()

RPG Configuration module with magnitude dictionary

return {
	Magnitudes = {
		[15] = {
			["StunTime"] = 0.7,
			["Damage"] = 150,
			["ConcussionEnabled"] = true,
			["Ragdoll"] = true,
			["RagdollTime"] = 2.5,
			["BrutalDeath"] = true,
			["Force"] = 50
		},
		[25] = {
			["StunTime"] = 0.46,
			["Damage"] = 65,
			["ConcussionEnabled"] = true,
			["Ragdoll"] = true,
			["RagdollTime"] = 1.5,
			["Force"] = 30
		},
		[35] = {
			["StunTime"] = 0.2,
			["Damage"] = 45,
			["ConcussionEnabled"] = true,
			["Ragdoll"] = true,
			["RagdollTime"] = 0.5,
			["Force"] = 15
		},
	},
	MaximumDistance = 35,
	RocketName = "RPG_Rocket",
	Speed = 200,
	RocketRotVel = 3, 
	UpF = 240,
	StartTime = 0.05,
	MaximumTime = 7,
	Stages = {
		Enabled = true,
		Time = 1,
		DamageIncrease = 30,
		Magnitudes = {
			[25] = {
				["StunTime"] = 3,
				["Damage"] = 150
			},
			[45] = {
				["StunTime"] = 2,
				["Damage"] = 70
			},
		},
	}
}
1 Like
#2

Denest your code a bit to make it simpler to understand.
The most simplest way to denest is to invert your if statements and switch to an early return. Also known as guard clause.

Also RisytalModules:ReturnDistance(v, Rocket) <= Config.MaximumDistance is the same as (v.PrimaryPart.Position - Rocket.Position).Magnitude <= Config.MaximumDistance which you’ve already checked, so you can remove one of them.

The code for NPCs and players is exactly the same for as the one for players, so you can just put the same logic in 1 if statement. (which then you can just invert and switch to early return)

There doesn’t seem to be any reason to add a coroutine, since the code doesn’t yield anyways.

With these changes I’ve simplified your code a bit:

local Connection
Connection = Rocket.Touched:Connect(function(Hit: BasePart)
    if Hit:IsDescendantOf(tool.Parent) then
        return
    end

    Touched = true
    Connection:Disconnect()
    Rocket.Anchored = true
    Rocket.Transparency = 1
    Rocket:ClearAllChildren()

    for _, child in pairs(workspace:GetChildren()) do
        if not (child:IsA("Model") or child:FindFirstChild("Humanoid")) then
            continue
        end

        local withinDistance = RisytalModules:ReturnDistance(child, Rocket) <= Config.MaximumDistance
        local character = game:GetService("Players"):GetPlayerFromCharacter(child) or child:GetAttribute("IsDummy")
        if not (withinDistance or character) then
            continue
        end

        local Magnitude = FindInterval(RisytalModules:ReturnDistance(child, Rocket), Config)
        child.Humanoid:TakeDamage(Magnitude.Damage)

        if Magnitude.Ragdoll then
            RisytalModules:ReturnF(child, "ev").Ragdoll:Fire(2.5)
            FlingP(Rocket, child.PrimaryPart, Magnitude.Force, Magnitude.StunTime)
        end

        if not Magnitude.BrutalDeath then
            continue
        end

        task.spawn(function()
            for _, value in pairs(child.ValuesFolder.LimbHealths:GetChildren()) do
                if tostring(value):match("Head") then
                    continue
                end
                value.Value = 0
            end
        end)
    end

    Rocket:Destroy()
    Rocket = nil   
end)
#3

so sorry for late response but is there a way to get player if character is not a dummy?