Dummy autospwan on certain spwan points

As the title says, I worked on a dummy auto spwan system onto a specific point on a grind if it dies. I want the code reviewed to ensure no memory leaks or performance issues on the server. If there’s anything that can be improved, please do tell me.

Grid:
image

Spwan points:
image

Code:

--!strict
local SS = game:GetService("ServerStorage")

local Spwaner = workspace.Spwaner
local SpwanPoints = Spwaner.SpwanPoints
local DummiesFolder = Spwaner.Dummies

local DummiesStore = SS.Dummies
local DummyCopy = DummiesStore.Dummy

local Connections:{[Model]:RBXScriptConnection} = {}
local RespwanDelay = 1.5

local GetAvailableSpots = function() : {BasePart}
	local Spots = {}
	
	for I, X in pairs(SpwanPoints:GetChildren()) do
		if not X:IsA("BasePart") then continue end 
		
		local Params = OverlapParams.new()
		Params.FilterDescendantsInstances = {workspace.SpecialRigs}
		Params.MaxParts = 9
		Params.FilterType = Enum.RaycastFilterType.Exclude
		
		local Cast = workspace:GetPartBoundsInBox(X.CFrame, X.Size, Params)
		local SpotOpen = true
		
		for II, XX in pairs(Cast) do
			local parent = XX.Parent
			
			if parent.Name == "Dummy" and parent:FindFirstChildOfClass("Humanoid") and parent.Parent == DummiesFolder then SpotOpen = false break end
		end
		
		if SpotOpen then 
			table.insert(Spots, X)
		end
	end
	
	return Spots
end

local SpwanDummy = function(SpwanLocation:CFrame) : Model?
	if typeof(SpwanLocation) ~= "CFrame" then return nil end
	
	local CopiedDummy = DummyCopy:Clone()
	CopiedDummy.HumanoidRootPart.CFrame = SpwanLocation
	CopiedDummy.Parent = DummiesFolder
	
	return CopiedDummy
end

local RefreshSpwans;
	
RefreshSpwans = function()
	for I, X in pairs(GetAvailableSpots()) do
		if not X:IsA("BasePart") then continue end

		local NewDummy = SpwanDummy(X.CFrame)
		if not NewDummy then continue end

		local NewHum = NewDummy:FindFirstChildOfClass("Humanoid")
		if not NewHum then continue end

		local Connection:RBXScriptConnection;

		Connection = NewHum.Died:Once(function() 
			task.wait(RespwanDelay)


			NewDummy.Parent = nil
			NewDummy:Destroy()

			RefreshSpwans()

			Connections[NewDummy]:Disconnect()
			Connections[NewDummy] = nil
		end)

		Connections[NewDummy] = Connection
		break
	end
end

local function Init()
	for I, X in pairs(SpwanPoints:GetChildren()) do
		if not X:IsA("BasePart") then continue end
		
		local NewDummy = SpwanDummy(X.CFrame)
		if not NewDummy then continue end
		
		local NewHum = NewDummy:FindFirstChildOfClass("Humanoid")
		if not NewHum then continue end
		
		local Connection:RBXScriptConnection;
		
		Connection = NewHum.Died:Once(function() 
			task.wait(RespwanDelay)
			
			
			NewDummy.Parent = nil
			NewDummy:Destroy()
			
			RefreshSpwans()
			
			Connections[NewDummy]:Disconnect()
			Connections[NewDummy] = nil
		end)
		
		Connections[NewDummy] = Connection
		task.wait()
	end
end

Init()
1 Like

Connection:Once() Will automatically disconnect the function after completing it, meaning that you’re safe to just remove all of the connection references and stuff. (Also when you destroy the humanoid, all associated connections will be removed furthering that)

You should be fine to remove that entire Connections table and anything that uses it.


The RefreshSpwans function is almost the same as the Init function, so I’d probably just have the init function either use the RefreshSpawns function (since there will be no dummies existing at that time) or add an argument in order to force spawn all the dummies. That’ll help to cut down on some copy and paste.


Outside of that, I’d also recommend considering to use different variable names for the for loops. Since I, X, II, and XX can be a bit confusing when reading the code down the line.

for I, X in pairs(SpwanPoints:GetChildren()) do
    for II, XX in pairs(Cast) do
         local parent = XX.Parent
    end
end

for _, Point in pairs(SpwanPoints:GetChildren()) do -- Part would also be okay too
    for _, Hit in pairs(Cast) do
         local parent = Hit.Parent
    end
end

It just makes things clearer regarding the intention of the code and what the variable might be.

1 Like

Everything you said was valid and will be implemented but the Init function and the RefreshSpwans are there because the Init function spwans in the dummies onto the spwan point location and when one of the dummies dies I have to create one and establish the connections again so i use the RefreshSpwans function as it does the same thing as the Init function but it only does it for one.

Also, to be more clear on my intention of having the RefreshSpwans be the same but spwaning one is because theoretically you can push all the dummies out of the way then kill one dummy which would double the amount of dummies ( Since it uses get parts in bound to detect if a spwan place is occupied)so i figured one spwaned for every one dummy killed would solve this. But if you got recommendations to solve this ill be willing to hear.

Shouldn’t spawn be the correct spelling?


You should simplify this code:
local RefreshSpwans;
	
RefreshSpwans = function()

to the one liner:

local function RefreshSpwans()

Third, I recommend following https://roblox.github.io/lua-style-guide/ for overall code style.

For all other points follow @kingerman88’s advice

Ah I see, I didn’t notice the break at the bottom of the for loop there. I’d probably recommend changing that since having a for loop and breaking after one iteration can cause confusion (using myself as the example I didn’t see it and assumed that the code was the same as the init function).

This should work functionally the same but doesn’t use the loop

local RespawnDummy
function RespawnDummy()
	local Spots = GetAvailableSpots()
	local Point = Spots[1] 
    -- alternatively: local Point = Spots[math.random(1, #spots)]
    -- if you want a random point to be selected

	local NewDummy = SpwanDummy(X.CFrame)
	local NewHum = NewDummy:FindFirstChildOfClass("Humanoid")
	if not NewHum then return end

	NewHum.Died:Once(function()
		task.wait(RespwanDelay)
		NewDummy:Destroy()
		RespawnDummy()
	end)
end

Also changing the name to RespawnDummy shows clear intention of the function, where RefreshSpawns is more ambiguous which can lead readers to think that you’re respawning at all spawns (plural)

My reason doing that is simple: you can’t recall functions unless they’re already declared. That’s what i was doing

I’m not gonna lie I’m so bad at variable naming that I named variables in a script just fruit names. But anyway back on topic, im sure if the table didn’t return anything and you tried to index it would error wouldn’t it?

There isn’t any difference. They behave exactly in the same way

True

Just add a guard clause that checks if the point is nil

You can’t use a function in a function unless its pre-declared if im right. And im sure even if it did work it autocomplete wouldn’t see it that way