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:
Spwan points:
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()
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.
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.
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)
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?
