Help optimizing my part selection system

Guest_2000123145 · May 1, 2024, 1:45am

Two things:

My fps drops to around 40 and stays there for a few seconds after the game starts, and disabling the script fixes it, so I think it has to do something with the script.
My fps drops to around 30 when quickly selecting parts, but I think this is only caused by the Instance.new("SelectionBox"), because once they’re all in the ‘cache’ (once they’re all generated), it stops lagging.

local UserInputService = game:GetService("UserInputService")

local Player = game.Players.LocalPlayer
local Mouse = Player:GetMouse()

local Selected = {}
local Boxes = { InUse = {} , Cache = {} }

local function HightlightPart(part)
	local box = table.remove(Boxes.Cache, 1) -- retrieves an unused selection box if there is one

	if not box then -- creates a box if there wasn't one in the cache
		box = Instance.new("SelectionBox")
		box.LineThickness = 0.05
		box.Adornee = part
		box.Parent = workspace
	end
	box.Adornee = part

	table.insert(Boxes.InUse, box)
end

local function UnhightlightPart(part)
	local position, box
	for i,selection in pairs(Boxes.InUse) do
		if selection.Adornee == part then
			position = i
			box = selection
		end
	end

	if box then
		box.Adornee = nil
		table.insert(Boxes.Cache, table.remove(Boxes.InUse, position))
	end
end

Mouse.Button1Down:Connect(function()
	
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 
	local check = table.find(Selected, Target) 

	if Target and Target.Parent then

		if SelectMultiple then
			
			if check then
				UnhightlightPart(Target)
				table.remove(Selected, check) 
			else
				HightlightPart(Target)
				table.insert(Selected, Target)
			end
			
		elseif (check and #Selected>1) or (not check) then -- deselects all except the one that was just clicked
			for _,part in pairs(Selected) do
				UnhightlightPart(part)
			end
			table.clear(Selected)

			HightlightPart(Target)
			table.insert(Selected, Target)
		elseif check then -- deselects only the clicked one
			UnhightlightPart(Target)
			table.remove(Selected, check)
		end

	elseif not SelectMultiple then 
		for _,part in pairs(Selected) do
			UnhightlightPart(part)
		end
		table.clear(Selected)
	end

end)

Put this in a localscipt under StarterPlayerScripts (the character doesn’t spawn in my game)

Thanks!

Katrist · May 2, 2024, 2:53am

I believe you overengineered it by adding a cache for the selection boxes. You also should’ve used a dictionary instead of an array because it would perform faster and easier to implement.

Code:

local UserInputService = game:GetService("UserInputService")

local Player = game.Players.LocalPlayer
local Mouse = Player:GetMouse()

local Boxes = {}

local function HightlightPart(part)
	local box = Instance.new("SelectionBox")
	box.LineThickness = 0.05
	box.Adornee = part
	box.Parent = workspace

	Boxes[part] = box
end

local function UnhightlightPart(part)
	Boxes[part]:Destroy()
	Boxes[part] = nil
end

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if Target and Target.Parent then
		local check = Boxes[Target]

		if SelectMultiple then

			if check then
				UnhightlightPart(Target)
			else
				HightlightPart(Target)
			end

		else -- deselects all except the one that was just clicked		
			for part in Boxes do
				UnhightlightPart(part)
			end

			HightlightPart(Target)
		end

	elseif not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end

end)

Guest_2000123145 · May 2, 2024, 3:29am

This almost works. How do I get the length of Boxes? #Boxes doesn’t work for some reason

Thanks!

Katrist · May 2, 2024, 3:57am

Why do you need to get the length of boxes?

Guest_2000123145 · May 2, 2024, 4:05am

It’s hard to explain, but look at my original post, press ctrl f and look up ‘#’.

Katrist · May 2, 2024, 1:58pm

That part was unnecessary. You can see that both scripts function the same.

You may have gotten my old script because I edited it a few times after I replied to improve it.

Guest_2000123145 · May 2, 2024, 3:38pm

It is not unnecessary. There’s a reason why I made it so complicated. Any way I can see the improvements you made? Thanks!

Katrist · May 2, 2024, 10:41pm

Just check my post. All I did was edit my reply.

Guest_2000123145 · May 2, 2024, 10:47pm

I can’t really notice any changes. Anyway, do you know why #Boxes doesn’t work? It always outputs 0. Thanks!

Katrist · May 2, 2024, 11:04pm

Sorry for asking for the third time but why do you need to know the length of a dictionary? The functionality in your original code seems to exactly match the one in mine, so I have 0 clue what needs to be changed.

Guest_2000123145 · May 2, 2024, 11:14pm

I can bet a lot of money that the functionality of your code does not exactly match my original
No worries though, I can show you
open up a new world and put my script into StarterPlayerScripts
and then spawn a bunch of parts in side by side
hold shift, and select all of them with my script, and then let go of shift, and click on an already selected part. All of them deselect but the part I just clicked
yours doesn’t do that
Thanks!

Katrist · May 2, 2024, 11:19pm

I was just asking, as you didn’t answer my question three times in a row.

Here’s a video demonstrating exactly what you said, except for the fact that I’m using my script:

External Media

I don’t know if you didn’t test the script, or if you just assumed it didn’t work.

Guest_2000123145 · May 2, 2024, 11:27pm

Wow! You’re right! Sorry about that! I assumed it didn’t work because I couldn’t really see any changes to it. Guess I lost money on that
Thank you so much!

ZerroxShiot · May 3, 2024, 1:09pm

I still feel like the logic could be simplified, especially at:

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if Target and Target.Parent then
		local check = Boxes[Target]

		if SelectMultiple then
			if check then
				UnhightlightPart(Target)
			else
				HightlightPart(Target)
			end
		else
			for part in Boxes do
				UnhightlightPart(part)
			end
			HightlightPart(Target)
		end
	elseif not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end
end)

if SelectMultiple is false it deselects all highlights in all cases. So if we move it a bit we get:

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end

	if Target and Target.Parent then
		local check = Boxes[Target]

		if SelectMultiple then
			if check then
				UnhightlightPart(Target)
			else
				HightlightPart(Target)
			end
		else
			HightlightPart(Target)
		end
	end
end)

That’s better, but if we add function named ToggleHighlight we could condense the code further.

local function ToggleHighlight(part)
	if Boxes[part] then
		RemoveHighlight(part)
	else
		AddHighlight(part)
	end
end

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end

	if Target and Target.Parent then
		if SelectMultiple then
			ToggleHighlight(Target)
		else
			HightlightPart(Target)
		end
	end
end)

But now HighlightPart could be replaced with ToggleHighlight since all parts are deselected if we are not selecting multiple which would highlight the part when used with ToggleHighlight.

local function ToggleHighlight(part)
	if Boxes[part] then
		RemoveHighlight(part)
	else
		AddHighlight(part)
	end
end

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end

	if Target and Target.Parent then
		ToggleHighlight(Target)
	end
end)

The logic could still be streamlined a bit better by moving the loop into its own function:

local function UnhighlightAllParts()
	for part in Boxes do
		UnhighlightPart(part)
	end
end

and with using a bit of Roblox Lua Style guide for naming and styling with some generally better naming we get:

local UserInputService = game:GetService("UserInputService")
local Players = game:GetService("Players")

local localPlayer = Players.LocalPlayer
local mouse = localPlayer:GetMouse()
local boxes = {}

local function addHighlight(part)
	boxes[part] = Instance.new("SelectionBox")
	boxes[part].LineThickness = 0.05
	boxes[part].Adornee = part
	boxes[part].Parent = workspace
end

local function removeHighlight(part)
	boxes[part]:Destroy()
	boxes[part] = nil
end

local function toggleHighlight(part)
	if boxes[part] then
		removeHighlight(part)
	else
		addHighlight(part)
	end
end

local function removeAllHighlights()
	for part in boxes do
		removeHighlight(part)
	end
end

mouse.Button1Down:Connect(function()
	local target = mouse.Target
	local isSelectingMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift)

	if not isSelectingMultiple then
		removeAllHighlights()
	end

	if target and target.Parent then
		toggleHighlight(target)
	end
end)

Guest_2000123145 · May 3, 2024, 5:00pm

is this better for fps? I feel like it might be slightly worse cuz there’s more functions to jump around to (if that makes sense). Because that’s my main focus, as I kinda want my friend to be able to play at 60 fps, and he has a laptop from the mid 2000s

Thanks!

Guest_2000123145 · May 3, 2024, 5:04pm

oh, I also just realized something! When you click on an already selected part, but you aren’t holding the leftshift down, it doesn’t deselect it. That might be the only difference between yours and mine. Thanks!

Edit: There’s actually no need for that, but Imma just point that out

Guest_2000123145 · May 3, 2024, 5:18pm

Oh, and by the way guys, there’s a limit on how many parts can be highlighted at a time (apparently 31) so there needs to be a way to limit it. Since this is waaay above my paygrade, I’d like y’all to figure out how to implement that, because I feel like anything I’d do would just ruin it :р
https://create.roblox.com/docs/reference/engine/classes/Highlight

or maybe this is just for the highlight, and not the box selection.

Edit: apparently it is just for the highlight. cool

Edit 2: I’d like to display how many I have selected on a ui thingie, and for some reason #boxes doesn’t work, so I’ll go with this:

is this good?
Either way, thank y’all both!

Katrist · May 3, 2024, 9:38pm

Yep that’s good.

Guest_2000123145 · May 3, 2024, 9:43pm

Perfect! Thank you so much!

(30 chаracters)

Katrist · May 3, 2024, 9:52pm

Actually, I just realized a more optimized solution.

The problem with counting like that is you’ll have to loop through every single instance in the dictionary to get a count, which is inefficient. A more optimized way would just to add and subtract from a counter variable.

Code:

local UserInputService = game:GetService("UserInputService")

local Player = game.Players.LocalPlayer
local Mouse = Player:GetMouse()

local boxesCount = 0
local Boxes = {}

local function HightlightPart(part)
	boxesCount += 1
	
	local box = Instance.new("SelectionBox")
	box.LineThickness = 0.05
	box.Adornee = part
	box.Parent = workspace

	Boxes[part] = box
end

local function UnhightlightPart(part)
	boxesCount -= 1
	
	Boxes[part]:Destroy()
	Boxes[part] = nil
end

Mouse.Button1Down:Connect(function()
	local Target = Mouse.Target
	local SelectMultiple = UserInputService:IsKeyDown(Enum.KeyCode.LeftShift) 

	if Target and Target.Parent then
		local check = Boxes[Target]

		if SelectMultiple then

			if check then
				UnhightlightPart(Target)
			else
				HightlightPart(Target)
			end

		else -- deselects all except the one that was just clicked		
			for part in Boxes do
				UnhightlightPart(part)
			end

			HightlightPart(Target)
		end

	elseif not SelectMultiple then 
		for part in Boxes do
			UnhightlightPart(part)
		end
	end

	print(boxesCount)
end)