So I am currently working on a spawn system for my building game that I am working on, this code does work, however it is kinda slow, I’m wondering if there’s a way it could be faster?
It can be faster if I remove the wait in the IsValidWeld, however then it would just freeze the client for a bit, and I’m afraid that would just disconnect them.
game.ReplicatedStorage.Remotes.Spawn.OnServerInvoke = function(Player)
local success, result = pcall(function()
local Plot = GetPlayerPlot(Player)
if not Plot then
return false
end
local BuildName = Player.Name .. tostring(Player.UserId) .. "_Build"
local PlotName = tostring(Player.UserId) .. "_Plot"
if not game.Workspace:FindFirstChild(BuildName) then
if not game.ServerStorage:FindFirstChild(PlotName) then
local Folder = Instance.new("Folder", game.ServerStorage)
Folder.Name = PlotName
end
local Build = Instance.new("Model", game.Workspace)
Build.Name = BuildName
for _,v in ipairs(Plot:GetChildren()) do
if v:IsA("Model") then
v.Parent = game.ServerStorage[PlotName]
local CopyModel = v:Clone()
CopyModel.Parent = Build
end
end
local function IsWeldValid(P0, P1)
wait()
for _,v:WeldConstraint in ipairs(Build:GetChildren()) do
if v:IsA("WeldConstraint") then
if (v.Part0 == P0 and v.Part1 == P1) or (v.Part0 == P1 and v.Part1 == P0) then
return false
end
end
end
return true
end
for _,v:Model in ipairs(Build:GetChildren()) do
if v:IsA("Model") then
local CF, Size = v:GetBoundingBox()
local Params = OverlapParams.new()
Params.FilterDescendantsInstances = {v}
Params.FilterType = Enum.RaycastFilterType.Exclude
local PartsInBlock = workspace:GetPartBoundsInBox(CF, Size - Vector3.new(1, 1, 1), Params)
for _,j in ipairs(PartsInBlock) do
if j:IsDescendantOf(Build) then
local MainModel = j:IsA("Model") and j or j:FindFirstAncestorOfClass("Model")
if not IsWeldValid(v.Main, MainModel.Main) then
local Weld = Instance.new("WeldConstraint")
Weld.Parent = Build
Weld.Part0 = v.Main
Weld.Part1 = MainModel.Main
end
end
end
for _,j:Model in ipairs(Build:GetChildren()) do
if j:IsA("Model") then
if j ~= v then
for _,k:Attachment in ipairs(j.PrimaryPart:GetChildren()) do
if k:IsA("Attachment") and k.Name == "ConnectingPoint" then
for _,l:Attachment in ipairs(v.PrimaryPart:GetChildren()) do
if l:IsA("Attachment") and l.Name == "ConnectingPoint" then
if l.WorldCFrame.Position == k.WorldCFrame.Position then
if IsWeldValid(v.Main, j.Main) then
local Weld = Instance.new("WeldConstraint")
Weld.Parent = Build
Weld.Part0 = v.Main
Weld.Part1 = j.Main
end
end
end
end
end
end
end
end
end
end
end
for _,v in ipairs(Build:GetDescendants()) do
if v:IsA("BasePart") or v:IsA("UnionOperation") then
if v.Name == "PlacementBlock" then
v:Destroy()
else
v.Anchored = false
end
end
end
return true
else
return false
end
end)
if not success then
print(result)
return false
else
return result
end
end```
Please separate your code into multiple functions, and use and instead of spamming if.
Also, instead of keeping executed code inside the if-statement, just turn that if-statement into a negation, and if that negation is true, use continue to not run the next bit of code.
My bad about the end hell thing, I completely forgot continue existed. I cleaned some of it up, however I’m not entirely sure what you mean about separating it into multiple functions. Here’s the new code fo it.
game.ReplicatedStorage.Remotes.Spawn.OnServerInvoke = function(Player)
local success, result = pcall(function()
local Plot = GetPlayerPlot(Player)
local BuildName = Player.Name .. tostring(Player.UserId) .. "_Build"
local PlotName = tostring(Player.UserId) .. "_Plot"
if not Plot or game.Workspace:FindFirstChild(BuildName) then return false end
if not game.ServerStorage:FindFirstChild(PlotName) then
local Folder = Instance.new("Folder", game.ServerStorage)
Folder.Name = PlotName
end
local Build = Instance.new("Model", game.Workspace)
Build.Name = BuildName
for _,v in ipairs(Plot:GetChildren()) do
if not v:IsA("Model") then continue end
v.Parent = game.ServerStorage[PlotName]
local CopyModel = v:Clone()
CopyModel.Parent = Build
end
local function IsWeldValid(P0, P1)
for _,v:WeldConstraint in ipairs(Build:GetChildren()) do
if not v:IsA("WeldConstraint") then continue end
if (v.Part0 == P0 and v.Part1 == P1) or (v.Part0 == P1 and v.Part1 == P0) then
return false
end
end
return true
end
for _,v:Model in ipairs(Build:GetChildren()) do
if not v:IsA("Model") then continue end
local CF, Size = v:GetBoundingBox()
local Params = OverlapParams.new()
Params.FilterDescendantsInstances = {v}
Params.FilterType = Enum.RaycastFilterType.Exclude
local PartsInBlock = workspace:GetPartBoundsInBox(CF, Size - Vector3.new(1, 1, 1), Params)
for _,j in ipairs(PartsInBlock) do
if not j:IsDescendantOf(Build) then continue end
local MainModel = j:IsA("Model") and j or j:FindFirstAncestorOfClass("Model")
if not IsWeldValid(v.Main, MainModel.Main) then continue end
local Weld = Instance.new("WeldConstraint")
Weld.Parent = Build
Weld.Part0 = v.Main
Weld.Part1 = MainModel.Main
wait()
end
for _,j:Model in ipairs(Build:GetChildren()) do
if not j:IsA("Model") or j == v then continue end
for _,k:Attachment in ipairs(j.PrimaryPart:GetChildren()) do
if not k:IsA("Attachment") or k.Name ~= "ConnectingPoint" then continue end
for _,l:Attachment in ipairs(v.PrimaryPart:GetChildren()) do
if (not l:IsA("Attachment") or l.Name ~= "ConnectingPoint") or (l.WorldCFrame.Position ~= k.WorldCFrame.Position) then continue end
if not IsWeldValid(v.Main, j.Main) then continue end
local Weld = Instance.new("WeldConstraint")
Weld.Parent = Build
Weld.Part0 = v.Main
Weld.Part1 = j.Main
wait()
end
end
end
end
for _,v in ipairs(Build:GetDescendants()) do
if not v:IsA("BasePart") and not v:IsA("UnionOperation") then continue end
if v.Name == "PlacementBlock" then
v:Destroy()
else
v.Anchored = false
end
end
return true
end)
if not success then
print(result)
return false
else
return result
end
end
Out of curiosity, why do you have a wait() in your code, that will certainly slow things down.
for _,j:Model in ipairs(Build:GetChildren()) do
if not j:IsA("Model") or j == v then continue end
for _,k:Attachment in ipairs(j.PrimaryPart:GetChildren()) do
if not k:IsA("Attachment") or k.Name ~= "ConnectingPoint" then continue end
for _,l:Attachment in ipairs(v.PrimaryPart:GetChildren()) do
...
wait()
end
end
end
Not sure what that’s achieving. Are you getting an error for the script running too long so you added a wait?
for _,v in ipairs(Plot:GetChildren()) do
if not v:IsA("Model") then continue end
v.Parent = game.ServerStorage[PlotName]
local CopyModel = v:Clone()
CopyModel.Parent = Build
end
Why not just copy the Plot instead of copying every child independently here? I see you’re not copying models so I’m sort of curious what you’re copying into Build here… if your script is slow then you must be copying quite a lot, in which case it may actually be faster to copy the whole Plot and delete the models after cloning.
Aside from that, I’m not too sure what’s slow here, unless your Build is just an absolutely massive model.
I probably should’ve been more clear here, to answer your questions:
Without the wait() would be faster, however depending on how large their build is, it will freeze the client, and if frozen for too long, would probably disconnect them.
That is a great idea, while that part is not really the part that is doing the freezing, or at least most of it, that would be a better solution for copying the Plot, also if you look closely at the script, I am only copying models, it’s negated so if it’s not a model, it will continue the loop and ignore the rest of the code. Thanks to @iGottic for suggesting that.
Anyways referring to the part that is mainly doing the freezing is the weld for loop itself. Sorry if I’m still not very clear.
