Better way to do this?

Help and Feedback Scripting Support
#1

I have this code, it works fine. However I do not like that a function is being called every frame, but I don’t know another way to accomplish what I want, this is a local script for a single player shooter game and server does perform checks as to avoid exploiters.

Is having it call that function every frame a bad idea, will it hurt performance? In testing it doesn’t, but yet again I have a decent system so it might actually be bad and i just would not know

local highlite = game.ReplicatedStorage:WaitForChild('Highlight')
local player = game.Players.LocalPlayer
local char = player.Character or player.CharacterAdded:Wait()
local root = char:WaitForChild("HumanoidRootPart")

local UIS = game:GetService("UserInputService")

local RS = game:GetService("RunService")
local takeables = game.Workspace:WaitForChild("Takeable")
local takeItemEvent = game.ReplicatedStorage:WaitForChild("ImportantRemotes"):WaitForChild('TakeItem')

local pickupRange = 20

local LastHeighliteModel = nil
local LastHeighlite = nil


local RayParams = RaycastParams.new()
RayParams.FilterType = Enum.RaycastFilterType.Exclude

function lineOfSight(target)
	
	local rayDirection = target.Position - root.Position
	local rayParams = RaycastParams.new()
	rayParams.FilterDescendantsInstances = {root.Parent}  

	local RayResult = workspace:Raycast(root.Position, rayDirection, rayParams)

	if RayResult and RayResult.Instance then
		if RayResult.Instance:IsDescendantOf(target.Parent) then
			return true
		else
			return false
		end
	end
	
end


function getNearby()
	
	local WithinDistance = {}
	
	for i, v in pairs(takeables:GetDescendants()) do
		if v:IsA('Model') and v.PrimaryPart then
			local distance = (v.PrimaryPart.Position - root.Position).Magnitude
			if distance <= pickupRange and lineOfSight(v.PrimaryPart) then
				WithinDistance[v] = distance
			end
		end
	end
	
	if next(WithinDistance) ~= nil then
		local smallestDistance = 10000000000
		local closestModel = nil
		for i, v in pairs(WithinDistance) do
			if v < smallestDistance then
				smallestDistance = v
				closestModel = i
			end
		end
		return closestModel
		
	else
		return nil
	end
end

UIS.InputBegan:Connect(function(input, gp)
	if gp then return end
	if input.KeyCode ~= Enum.KeyCode.T then return end
	
	local itemToTake = getNearby()
	
	if itemToTake ~= nil then
		takeItemEvent:FireServer(itemToTake)
	end
	
end)

RS.RenderStepped:Connect(function()
	local nearby = getNearby()

	if nearby ~= nil and LastHeighliteModel ~= nearby then
		if LastHeighlite then
			LastHeighlite:Destroy()
		end
		local newHiglite = highlite:Clone()
		newHiglite.Parent = nearby
		LastHeighliteModel = nearby
		LastHeighlite = newHiglite
	else
		if nearby == nil and LastHeighlite ~= nil then
			LastHeighlite:Destroy()
			LastHeighliteModel = nil
		end
	end
	
end)
#2

you’re getting the descendants for this takables variable. does this folder or model have a lot of parts? if not, it should be okay.

#3

Currently I’m just testing so it has like 4 models and not that many parts but I don’t know how big I’ll scale the game and thus how much more models are added

1 Like
#4

should still be fine, any chance you could lower how often you need to fire? like instead of every frame maybe every half second or smth?

1 Like
#5

It’s best to just spawn a bunch of takeables in and see how it effects performance.

IIRC Raycasts get more expensive the further away they are, so you might want to exclude takeables over a long distance away, like 500+ studs.

1 Like
#6

IIRC Raycasts get more expensive the further away they are, so you might want to exclude takeables over a long distance away, like 500+ studs.

Didn’t know that, I’ll check if the model’s primary part is within the max pickup distance from the player before performing a raycast