How to Optimize this Code

So I have a local environment script where it changes the current lighting everytime the camera is detected inside a region3.

	for i,v in pairs(game.Workspace.Environments:GetChildren()) do
		if v:IsA("Folder") then
			
			Found = false
			
			local min
			local max
			
			for i, planets in pairs(game.Workspace.PlanetarySystem:GetChildren()) do
				if planets:IsA("Model") then
					min = planets.Outer.PlanetarySurface.Position-(planets.Outer.PlanetarySurface.Size/2)*5
					max = planets.Outer.PlanetarySurface.Position+(planets.Outer.PlanetarySurface.Size/2)*5
				end
			end
			
			local region = Region3.new(min, max)
			
			if IsInRegion(game.Workspace.CurrentCamera.CFrame.Position,min,max) then
				Found = true
			end
			
			if Found == true then
				enter(v)
				for i,v in pairs(game.Workspace.PlanetarySystem:GetDescendants()) do
					if v.Parent.Name == "Tag" then
						if v.Enabled == true then
							v.Enabled = false
						end
					end
				end
				break
			else
				exit(v)
				for i,v in pairs(game.Workspace.PlanetarySystem:GetDescendants()) do
					if v.Parent.Name == "Tag" then
						if v.Enabled == false then
							v.Enabled = true
						end
					end
				end
			end
		end
	end
	wait(1)
end

This lags every time the while true do is ran. How could I optimize this?

1 Like

Just for analysis and minor improvements, based on the current scope of the code we have:

for _, directory in pairs(workspace.Environments:GetChildren()) do
	if not directory:IsA("Folder") then
		continue
	end

	Found = false -- This appears to be declared outside the scope

	local min
	local max

	for _, planet in pairs(workspace.PlanetarySystem:GetChildren()) do
		if planet:IsA("Model") then
			min = planet.Outer.PlanetarySurface.Position - (planet.Outer.PlanetarySurface.Size / 2) * 5
			max = planet.Outer.PlanetarySurface.Position + (planet.Outer.PlanetarySurface.Size / 2) * 5
		end
	end

	local region = Region3.new(min, max) -- This variable isn't being used, why name it?
	if IsInRegion(workspace.CurrentCamera.CFrame.Position, min, max) then
		-- Don't know what IsInRegion is...
		Found = true
	end

	if Found == true then
		enter(directory) -- Neither do I know this method
		-- [1] Code repeated
		for _, planet in pairs(workspace.PlanetarySystem:GetChildren()) do
			local tag = planet:FindFirstChild("Tag")
			if tag then
				-- Why bother checking if enabled? Just skip it and everything will be false
				tag = false
			end
		end
		break
	else
		exit(directory)
		-- [1] Code repeat
		for _, planet in pairs(workspace.PlanetarySystem:GetChildren()) do
			local tag = planet:FindFirstChild("Tag")
			if tag then
				-- Why bother checking if enabled? Just skip it and everything will be true
				tag = true
			end
		end
	end
end
task.wait(1)

For the final question: Why would you use a while true do loop for? What is so important about keeping it updated at a very fast rate?

2 Likes

Use Heartbeat instead of while true do

1 Like

I think you mean RunService.Heartbeat which is an event that syncs to frames, technically. I wonder why this refresh/update rate is necessary.

1 Like

task.wait(0.33) or just remove the wait all together.
Nothing to test, not sure what you’re asking for.
Format looks fine.

1 Like

Removing the wait in a while true do would crash the game.

1 Like

Figured one of your funtions had a stall also.
Do you need to see the update in real time? If not try lower than 0.33
I would say task.wait() … I just don’t like the way that looks, but it seems to work.

1 Like

It’s the only way I could think of that would update the lighting of the player. Since then I’ve been finding ways that wouldn’t run the function until the camera is found in the region3 (which, i couldn’t)

Wouldn’t this lag worse using RunService?

I’d not use loops for this. Perhaps check the new position of the camera when it’s moving. If that position is in the box, change lighting. Else, change lighting.

That way you can avoid having a loop run constantly

1 Like

So, currentcamera.Changed:Connect(function()?

Looking at the purpose of the script here, I’d have to agree. A triggered single pass would be just as good. Probably better.

I’d use GetPropertyChangedSignal. Instead of listening to a change on the camera (which has lots of properties) you could listen specifically to changing CFrame.

1 Like

When does it start lagging? When your camera enters or every second? Also how many parts are inside the folder your looping through? Those have a much bigger effect on lag than just having a while true do loop

Update. So I edited the code a bit from advice and this is extremely laggy. Whenever the camera doesn’t move it’s completely fine, and when it does it’s absolutely horrendous

game.Workspace.CurrentCamera:GetPropertyChangedSignal("CFrame"):Connect(function()
	for i,a in pairs(game.Workspace.PlanetarySystem:GetChildren()) do
		if a:IsA("Model") then
			for i,b in pairs(a:GetChildren()) do
				if b.Name == "Outer" then
					for i,c in pairs(b:GetChildren()) do
						if c.Name == "PlanetarySurface" then

							Found = false

							local min = c.Position-(c.Size/2)*5
							local max = c.Position+(c.Size/2)*5

							local region = Region3.new(min, max)

							if IsInRegion(game.Workspace.CurrentCamera.CFrame.Position,min,max) then
								Found = true
							end

							if Found == true then
								enter(c.Parent.Parent)
								for i,d in pairs(game.Workspace:GetDescendants()) do
									if d.Parent.Name == "Tag" then
										d.Enabled = false
									end
								end
								break
							else
								exit(c.Parent.Parent)
								for i,d in pairs(game.Workspace:GetDescendants()) do
									if d.Parent.Name == "Tag" then
										d.Enabled = true
									end
								end
							end
						end
					end
				end
			end
		end
	end
end)```
1 Like

In the previous piece of code used (the while true do loop) it lagged every one second, which is the wait interval for the loop.

Then It’s probably lagging because you’re looping through a ton of parts. Try creating 1 big part that covers the area you need and comparing the magnitude of that part to the camera position.

1 Like

The area where I need to cover is bigger than the part size limit, that’s why I used region3. I’m not really sure how to do that

then cache the position of a part in vector3 format and compare that position to the players position. It would basically be a circle radius though. Code might look something like this:

local Players = game:GetService("Players")

local localPlayer = Players.LocalPlayer
local partInWorldPosition = workspace.Part.Position

local circleRadius = 500

while true do
	if (partInWorldPosition - localPlayer.Character:GetPivot().Position).Magnitude < circleRadius then
		--If the player is inside the radius
	else
		--If the player is outside the radius
	end

	task.wait(1)
end

Note: Heartbeat and while true do are the exact same thing

You should use some sort of chunk system, instead of looping through the whole world, loop through the chunk and check the distance between the player & the “interactable” object, though distance checks are expensive too.

You’re creating a region every time the “PlanetarySurface” is found, perhaps create it once, it’s a map, so I don’t think the map will change throughout the game except the lightning.

Though this won’t do any changes, I recommend using workspace instead of game.Workspace, it exists specifically to save time.

Can you explain the “enter” & “exit” function?

3 Likes