Optimizing this destroypartfallheight implementation

Help and Feedback Code Review
1

currently i have a script that simulates destroypartfallheight, but it uses table.find even after a couple of checks and it runs every frame so it will be horribly laggy at large workspace sizes

	local pieces = {}
	local atty = {}
	for i, v in workspace.SpawnedRooms:GetDescendants() do
		if v:IsA("BasePart") then
			table.insert(pieces, v)
		end
		if v:IsA("Attachment") then
			table.insert(atty, v)
		end
	end

	local basis = CFrame.new()

	local centerLocal, sizeLocal = daft:computeBoundingBox(basis, pieces, atty)

	if sizeLocal.X <= 0 and sizeLocal.Y <= 0 and sizeLocal.Z <= 0 then
		return -- nothing to do
	end

	sizeLocal *= 30

	local half = sizeLocal * 0.5
	local minLocal = centerLocal - half
	local maxLocal = centerLocal + half

	local worldMin = basis:PointToWorldSpace(minLocal)
	local worldMax = basis:PointToWorldSpace(maxLocal)

	local regionMin = Vector3.new(
		math.min(worldMin.X, worldMax.X),
		math.min(worldMin.Y, worldMax.Y),
		math.min(worldMin.Z, worldMax.Z)
	)
	local regionMax = Vector3.new(
		math.max(worldMin.X, worldMax.X),
		math.max(worldMin.Y, worldMax.Y),
		math.max(worldMin.Z, worldMax.Z)
	)
	regionMax += Vector3.new(500, 500, 500)
	local notdeathregion = Region3.new(regionMin, regionMax)
	local thingie = workspace:FindPartsInRegion3(notdeathregion, nil, math.huge)
	for i, v in workspace:GetDescendants() do
		if v:IsA("BasePart") then
			if v.Anchored == false then
				if not table.find(thingie, v) then
					if v.Name == "Key" and v:FindFirstAncestorWhichIsA("Folder") then
						local x = keyspawner.Position.X + daft:randrand(-keyspawner.Size.X/2,keyspawner.Size.X/2, true)
						local y = keyspawner.Position.Y + daft:randrand(-keyspawner.Size.Y/2,keyspawner.Size.Y/2, true)
						local z = keyspawner.Position.Z + daft:randrand(-keyspawner.Size.Z/2,keyspawner.Size.Z/2, true)
						v.AssemblyLinearVelocity *= 0.5
						v.AssemblyAngularVelocity *= 0.5
						v.CFrame = CFrame.new(x, y, z)
						continue
					end
					if v.Parent:FindFirstChild("Humanoid") ~= nil then
						v.Parent.Humanoid.Health = 0
					elseif v.Anchored == true then
						continue
					else
						v:Destroy()
					end
				end
			end
		end
	end
1 Like
2

before you get to performance, you should focus on readability. not saying it’s illegible or anything but its really a minor change that can help define your code to other people.

now with the performance aspect:

use classname directly. it reduces the NAMECALL instruction.

store descendants as a variable. reason being is Luau can immediately run back to the scope the variable was in rather than running into the for loop without a previous reference.

local descendants = workspace.SpawnedRooms:GetDescendants()

for i = 1, #descendants do -- iterate in a numerical for loop instead.
	local v = descendants[i]
	local className = v.ClassName
	if className == "Part" or className == "MeshPart" or className == "UnionOperation" then
		table.insert(pieces, v)
	elseif className == "Attachment" then
		table.insert(atty, v)
	end
end

use numerical loops when you want to increase performance, as it significantly reduces the amount of bytecode instructions. it also is faster since it directly indexes numbers which Luau can easily optimize over method calls like :GetDescendants().

convert thingie into a set.

local availableParts = workspace:FindPartsInRegion3(notdeathregion, nil, math.huge)
for i = 1, #availableParts do
	availableParts [availableParts [i]] = true
end

when time comes for the for loop, you can do one smooth pass through the whole table. this reduces your lookup time to O(1)

-- CACHE!!! reduces bytecode instructions.
local keyX, keyY, keyZ = keyspawner.Position.X, keyspawner.Position.Y, keyspawner.Position.Z
local keySizeX, keySizeY, keySizeZ = keyspawner.Size.X / 2, keyspawner.Size.Y / 2, keyspawner.Size.Z / 2


local workspaceDescendants = workspace:GetDescendants()

for i = 1, #workspaceDescendants do
	local currentPart = workspaceDescendants[i] -- notice how much easier this is!
	if currentPart:IsA("BasePart") and not currentPart.Anchored and not availableParts[currentPart] then -- direct logic with the availabie parts table
		if currentPart.Name == "Key" and currentPart:FindFirstAncestorWhichIsA("Folder") then
			local x = keyX + daft:randrand(-keySizeX, keySizeX, true)
			local y = keyY + daft:randrand(-keySizeY, keySizeY, true)
			local z = keyZ + daft:randrand(-keySizeZ, keySizeZ, true)
			currentPart.AssemblyLinearVelocity *= 0.5
			currentPart.AssemblyAngularVelocity *= 0.5
			currentPart.CFrame = CFrame.new(x, y, z)
			continue	
		else
			local humanoid = currentPart.Parent:FindFirstChild("Humanoid")
			if humanoid then
				humanoid.Health = 0
			else
				currentPart:Destroy()
			end
		end
	end
end
1 Like
3

oh man i wonder whats slowing down my code
the devious tenacity to not use caching because it looks funnier

1 Like
4

yes i forget to cache all the time but its honestly ok because my game runs at 9 fps anyways

2 Likes
5

hold on how would i get the keys of two different tables into one, since the game is still lagging as hell when i do the stress test (aka a point which normal servers should reach)
image

1 Like
6

use a table merger

function merge(t1, t2)
	for i, v in t1 do
		if t2[i] then
			if type(v) == "table" then
				merge(v, t2[i])
			else
				t1[i] = t2[i]
			end
		end
	end
end
1 Like
7

i used

table.pack(table.unpack(workspace.Props:GetDescendants()), table.unpack(workspace.Keys:GetDescendants()))

is that fine enough? (i also made it so characters move to the workspace.Props folder on spawning)

8

your merge will crash because if the array size exceeds 7,900 you pass the stack limit.
mine also doesn’t work after some research since it directly overwrites t1.
so just use table.move.

local t1 = workspace.Props:GetDescendants()
local t2 = workspace.Keys:GetDescendants()
table.move(t2, 1, #t2, #t1 + 1, t1)

edit: both approaches are really slow as well. mine gives ~0.8ms with a wrong result and yours gives ~1.5ms or 0 because it crashes. table.move is ~0.08ms.

9

image
image762Ă—202 56.3 KB

i could move keys into props but then… the horrors would unleash, but screw it!!!

10

ok genuinely what could have possibly happened :skull:

11

if you assume the caption was related to the image, sorrgy wrong, that’s just what happened when usign table.move

12

this is probably because you called :GetDescendants() in the loop rather than outside of it.

local t1= workspace.Props:GetDescendants()
local t2= workspace.Keys:GetDescendants()
local t3= table.create(#propsDescendants + #keysDescendants)
table.move(t1, t1, #t1, 1, t3)
table.move(t2, 1, #t2, #t1+ 1, t3)

while true do
    for i = 1, #allDescendants do
        local v = allDescendants[i]
       -- process and print for testing ofc
       print(`processed {v.Name}`)
    end
    task.wait(0.1)
end

run this and see if it works

13

is there a event for when a descendant gets added i forgot (this could actually fix most of the issues with the code :eyes:)

14

the humble beings named .DescendantAdded and .DescendantRemoved:

eaa97a5c8df97c6f3117634bf287ccce
eaa97a5c8df97c6f3117634bf287ccce214Ă—235 8.32 KB

15

the devious tenacity to not use caching: YIKIESS!!! im oughta here
(anyways im moving alot of stuff outside of the loop and hooking them up to events :eyes:)

16 
local rs = game:GetService("RunService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local keyspawner = workspace.KeySpawn
local daft = require(ReplicatedStorage:WaitForChild("daft"))
local runtime = workspace.SpawnedRooms.StartingRoom.RunTime.SurfaceGui.Frame.Time
local spawnedrooms = workspace.SpawnedRooms
local props = workspace.Props
local pieces = {}
local atty = {}
local notdeathregion:Region3?
local propDescendants = props:GetDescendants()

function calcuatednotdeathregion()
	local basis = CFrame.new()

	local centerLocal, sizeLocal = daft:computeBoundingBox(basis, pieces, atty)

	if sizeLocal.X <= 0 and sizeLocal.Y <= 0 and sizeLocal.Z <= 0 then
		return -- nothing to do
	end

	sizeLocal *= 30

	local half = sizeLocal * 0.5
	local minLocal = centerLocal - half
	local maxLocal = centerLocal + half

	local worldMin = basis:PointToWorldSpace(minLocal)
	local worldMax = basis:PointToWorldSpace(maxLocal)

	local regionMin = Vector3.new(
		math.min(worldMin.X, worldMax.X),
		math.min(worldMin.Y, worldMax.Y),
		math.min(worldMin.Z, worldMax.Z)
	)
	local regionMax = Vector3.new(
		math.max(worldMin.X, worldMax.X),
		math.max(worldMin.Y, worldMax.Y),
		math.max(worldMin.Z, worldMax.Z)
	)
	regionMax += Vector3.new(500, 500, 500)
	notdeathregion = Region3.new(regionMin, regionMax)
end

for i, v in spawnedrooms.StartingRoom:GetDescendants() do -- no more numerical for loop!
	local className = v.ClassName
	if className == "Part" or className == "MeshPart" or className == "UnionOperation" then
		table.insert(pieces, v)
	elseif className == "Attachment" then
		table.insert(atty, v)
	end
end
calcuatednotdeathregion()



spawnedrooms.ChildAdded:Connect(function(child: Instance) 
task.wait() -- wait for the stuff in child to load
local descends = child:GetDescendants()

	for i = 1, #descends do -- iterate in a numerical for loop instead.
		local v = descends[i]
		local className = v.ClassName
		if className == "Part" or className == "MeshPart" or className == "UnionOperation" then
			table.insert(pieces, v)
		elseif className == "Attachment" then
			table.insert(atty, v)
		end
	end
	calcuatednotdeathregion()
end)

props.DescendantAdded:Connect(function(descendant: Instance) 
	propDescendants = props:GetDescendants()
end)

rs.PreSimulation:Connect(function(dt)
	local availableParts = workspace:FindPartsInRegion3(notdeathregion, nil, math.huge)
	for i = 1, #availableParts do
		availableParts [availableParts [i]] = true
	end
	local keyX, keyY, keyZ = keyspawner.Position.X, keyspawner.Position.Y, keyspawner.Position.Z
	local keySizeX, keySizeY, keySizeZ = keyspawner.Size.X / 2, keyspawner.Size.Y / 2, keyspawner.Size.Z / 2

	for i = 1, #propDescendants do
		local currentPart = propDescendants[i] -- notice how much easier this is!
		if currentPart:IsA("BasePart") and not currentPart.Anchored and not availableParts[currentPart] then -- direct logic with the availabie parts table
			if currentPart.Name == "Key" and currentPart:FindFirstAncestorWhichIsA("Folder") then
				local x = keyX + daft:randrand(-keySizeX, keySizeX, true)
				local y = keyY + daft:randrand(-keySizeY, keySizeY, true)
				local z = keyZ + daft:randrand(-keySizeZ, keySizeZ, true)
				currentPart.AssemblyLinearVelocity *= 0.0833333333
				currentPart.AssemblyAngularVelocity *= 0.0833333333
				currentPart.CFrame = CFrame.new(x, y, z)
				continue	
			else
				local humanoid = currentPart.Parent:FindFirstChild("Humanoid")
				if humanoid then
					humanoid.Health = 0
				else
					currentPart:Destroy()
				end
			end
		end
	end
end)
17

I don’t understand why you have to iterate all parts in the workspace. Why not just chunk out the region directly below your destroy threshold. Call GetPartBoundsInBox in this region. Returns a much smaller table of parts for you to iterate and check. Iterate over these parts to determine candidacy for destruction.

Even if this table is not much smaller:
You could consider implementing multithreading to filter this table because it seems the most expensive parts of your code involve iterating large tables.

18

because i don’t know how to get cframe from the bounding box :​(

19

Not sure what you mean?

if you want to destroy parts below y=-50, with 0<=x<=250, 0<=y<=250,

partsInRegion=workspace:GetPartBoundsInBox(CFrame.new(125,-75,125),Vector3.new(250,50,250))

First param CFrame describes origin of box to scan, second describes size of box

20

that wouldn’t work because i want the death barrier to be dynamic, also i tried just dropping in getboundsinbox with region3.cframe and size but it just instakills the player