Help improving this code

Help and Feedback Code Review
#1

Hello everyone,

i was making a script that would break all weldconstraints inside of an part at the mouses position and fling any parts inside away.

function module.DetectBox(Box, Velocity : number)	
	local Querry = workspace:GetPartsInPart(Box)
-------------------------------------------
	for Index, Part in Querry do
		----------
		local AssemblyVel = CFrame.lookAt(Box["Position"], Part["Position"])["LookVector"] * Velocity
		local AssemblyAng = -(Box["Position"] - Part["Position"]).Unit * (Velocity/5)
		----------
		if not Part["Anchored"] then
			for _, Constraint in pairs(Part:GetChildren()) do
				if Constraint:IsA("WeldConstraint") then
					Constraint:Destroy()
					print(Constraint)
				end
			end

			Part["AssemblyLinearVelocity"] = AssemblyVel
			Part["AssemblyAngularVelocity"] = AssemblyAng
		end---
	end-------
end-----------

the problem is that it is very laggy. probably a .5 second delay after the hitbox has been spawned to the parts inside being flinged. this likely is linked to the for loop that destroys the constraints.

how would i optimize it?

1 Like
#2

First things first:

Readability

I really don’t know why would you index instances using [“name”]. They are instances, not tables. Yes, they inherit the behavior of tables, but they are not the same. As an example, you can do:

script["Destroy"](script)

Do you think this is very readable? This code is the same as you would normally call script:Destroy()

Line 10

This loop can be improved if you instead use GetJoints

for _, Constraint in pairs(Part:GetJoints()) do
	if Constraint:IsA("WeldConstraint") then
		Constraint:Destroy()
		print(Constraint)
	end
end

Delay

This is mainly caused caused by workspace:GetPartsInPart(), you can either replace it with GetPartBoundsInBox or take a look into Parallel luau

1 Like
#3

I just think it looks better. its just personal taste. does it reduce preformance?

Would it be a good idea to destroy every constraint?

is this really that much better? is so, ill switch.
also, i did some testing, and it seems like the delay seems to actually be caused by the physics solver. after it does the entire loop, (it took like 0.008 seconds, maybe i did the troubleshooting wrong but this isn’t that bad i dont think) and after applying assemblylinearvelocity it would slow down, as if applying it to multiple things at once was too much to handle quickly, ig. i tried creating linearvelocity instances instead, but those seemed to do the same thing, if not worse

1 Like
#4

Optimizations:

  • Early filtering
  • Efficient Velocity Calculation
  • Selective Use of pairs()
  • Avoid Excessive Debugging Statements
function module.DetectBox(Box, Velocity : number)
    local Querry = workspace:GetPartsInPart(Box)

    -- Optimized loop over parts in query
    for _, Part in pairs(Querry) do
        -- Early filtering to skip anchored parts
        if not Part.Anchored then
            local AssemblyVel = (Box.Position - Part.Position).Unit * Velocity
            local AssemblyAng = (Box.Position - Part.Position).Unit * (Velocity / 5)

            -- Check if the part has WeldConstraints and break them
            local constraints = Part:GetChildren()
            for _, Constraint in pairs(constraints) do
                if Constraint:IsA("WeldConstraint") then
                    Constraint:Destroy()
                end
            end

            -- Apply velocity and angular velocity
            Part.AssemblyLinearVelocity = AssemblyVel
            Part.AssemblyAngularVelocity = AssemblyAng
        end
    end
end

If this doesn’t solve the problem for you then you might need to do Threading

1 Like
#5

is it really best to do:

local Wazzah = Part:GetChildren()
for _, foo in pairs(Wazzah) do

instead of

for _, foo in pairs(Part:GetChildren()) do

or is that simply personal preference??

#6

If no one else is going to read it then alright!

We do not destroy every constraint; we only return a table of the constraints that are using this part; we still have to filter needed constraints. This can provide a bit of performance since we only collect Constraints and not attachments or other instances under it

Yes, this information comes from roblox itself:

As noted, this spatial query method considers the exact volume occupied by the given part using a full geometric collision check. As an example, a concave/hollow part won’t match queried parts within it unless they actually overlap/touch such a part. For simpler volumes, consider using WorldRoot:GetPartBoundsInBox() or WorldRoot:GetPartBoundsInRadius(), as they are less accurate but perform more efficiently.