Optimizing a script indexing a lot of parts

Help and Feedback Code Review
#1

I have a function in my code that detects the closest node to the player, and if the current node is changed there is a separate value for the last closest node which the function updates aswell.

	local ngc = nodes:GetChildren()
	local nmags = {}
	for i, v in pairs(ngc) do
		table.insert(nmags,(playerpos*noyvector-v.Position*noyvector).Magnitude)
	end
	local lowest = math.min(unpack(nmags)) 
	for i, v in pairs(ngc) do
		if (playerpos*noyvector-v.Position*noyvector).Magnitude == lowest and cn.Value ~= v then
			if not cn.Value then
				cn.Value = v
				lcn = cn.Value
			else
				lcn = cn.Value
				cn.Value = v
			end
		end
	end

(noyvector is just Vector3.new(0,1,0))
The problem is that this function seems to take up a LOT of script activity: when it’s present the script activity is at 8-9%, but when I disable it the activity goes all the way down to 3%. I’m assuming this is because of the GetChildren() function which indexes a lot of parts (1012 to be exact).

Is there any way I could optimize this code?

4 Likes
#2

Lower the amount parts, I’m not exactly good with coding but what I’ve read possibly try that. If not, don’t listen to me.

3 Likes
#3

Not able to, the point of this post is asking if I could optimize the code without altering part amount

2 Likes
#4

If the number of nodes isn’t changing then I’d only call GetChildren once or when they do change and store that out of this scope.

Also, I’d use ipairs instead of pairs here since it’s by default faster

Though the majority of your issues are likely from iterating through all of the children and doing operations on each. There’s probably an algorithm for it somewhere to optimize

2 Likes
#5

The amount of nodes never changes

2 Likes
#6

Hey mate! Here’s a way I found to optimize the above code

	local nmags = {}
	local lowest = math.min(unpack(nmags)) 
	for i, v in pairs(nodes:GetChildren()) do
        table.insert(nmags,(playerpos*noyvector-v.Position*noyvector).Magnitude)
		if (playerpos*noyvector-v.Position*noyvector).Magnitude == lowest and v~= cn.Value  then
			if not cn.Value then
				cn.Value = v
				lcn = cn.Value
			else
				lcn = cn.Value
				cn.Value = v
			end
		end
     end
2 Likes
#7

I can already see this wouldn’t work: the nmags table is supposed to be ALL of the distances between the nodes and the player when the 2nd GetChildren() is ran, meanwhile in your script it adds in the distances at the same time as the check is being performed. Not to mention the lowest variable would return nil as there are no numbers in the nmags{} table beforehand, and it does not get updated whenever they are added into the table.

2 Likes
#8

Some optimisations in this code (with help from ChatGPT).

local ngc = nodes:GetChildren()
local lowest = math.huge
local cnValue = cn.Value

for i, v in ipairs(ngc) do
	local position = v.Position * noyvector
	local distance = (playerpos * noyvector - position).Magnitude
    
	if distance < lowest and cnValue ~= v then
		lowest = distance
		lcn = cnValue
		cn.Value = v
	end
end
2 Likes
#9

Because neyvector is just Vector3.new(0,1,0), you could just substitute (playerpos*noyvector-v.Position*noyvector).Magnitude with math.abs(playerpos.Y - node.Position.Y)

That being said, why are you calculating the magnitude again? Why not just use the following code:

local currentLowestMagnitude = math.huge
local currentNode = nil

for _, node in pairs(ngc) do
	local distance = math.abs(playerpos.Y - node.Position.Y)
    
	if distance < currentLowestMagnitude then
		currentLowestMagnitude = distance
		currentNode = node
	end
end

if not cn.Value then
	lcn = currentNode
	cn.Value = currentNode
else
	lcn = cn.Value
	cn.Value = currentNode
end

Also since you said

Make sure that you set this outside of the function: local ngc = nodes:GetChildren()

1 Like
#10

Make sure that you set this outside of the function: local ngc = nodes:GetChildren()

Did that shortly after making this post, doesn’t seem to change much
Also sorry for my mistake, noyvector is actually Vector3.new(1,0,1)

2 Likes
#11

How often are you calling this?

1 Like