Simple working module script may not be performant

local ArrowPart = game.ReplicatedStorage.Decoration.Arrow
local RunService = game:GetService("RunService")
local CollectionService = game:GetService("CollectionService")

local function GetRadius(Adornee)
	if not Adornee then return 0 end
	if Adornee.Parent:IsA("Model") then
		local Size = Adornee.Parent:GetExtentsSize()
		return math.max(Size.X, Size.Y, Size.Z)
	elseif Adornee:IsA("BasePart") then
		local Size = Adornee.Size
		return math.max(Size.X, Size.Y, Size.Z)
	end
	warn("No basepart or model")
	return 0
end

local function UpdateArrow(ArrowPart, Adornee, Point, Radius, Relative)
	if not ArrowPart or not ArrowPart.Parent then return end

	-- If Relative is true, Point is an offset from Adornee, otherwise it's a world position
	local targetPoint = Relative and (Adornee.Position + Point) or Point

	local direction = (targetPoint - Adornee.Position).Unit
	-- Position the arrow at adornee's position, offset by radius in the direction of point
	local arrowPosition = Adornee.Position + (direction * Radius)

	-- Instead of pointing AT targetPoint, create a CFrame that points in the direction
	-- by using a lookVector. We create a point far along the direction vector.
	local lookAtPoint = arrowPosition + (direction * 100) -- 100 studs away in that direction
	ArrowPart.CFrame = CFrame.new(arrowPosition, lookAtPoint) * CFrame.Angles(math.rad(-90), 0, 0)
end

------------------------
local Arrow = {}
Arrow.ArrowTable = {}

-- Single RenderStepped connection for ALL arrows
local updateConnection = nil

local function StartUpdateLoop()
	if updateConnection then return end -- Already running

	updateConnection = RunService.RenderStepped:Connect(function()
		-- Update all arrows in one loop
		for arrowPart, data in pairs(Arrow.ArrowTable) do
			if arrowPart and arrowPart.Parent and data._Adornee and data._Adornee.Parent then
				UpdateArrow(arrowPart, data._Adornee, data._Point, data._Radius, data._Relative)
			else
				-- Clean up invalid arrows
				Arrow.Remove(arrowPart)
			end
		end

		-- Stop the loop if no arrows remain
		if next(Arrow.ArrowTable) == nil then
			if updateConnection then
				updateConnection:Disconnect()
				updateConnection = nil
			end
		end
	end)
end

Arrow.Spawn = function(Adornee, Point, Relative)
	if not Adornee or not Point then error("Missing Adornee or Point") end

	Relative = Relative or false -- Default to false

	local NewArrowPart = ArrowPart:Clone()
	NewArrowPart.Parent = workspace
	NewArrowPart.Anchored = true
	NewArrowPart.CanCollide = false
	NewArrowPart.CanQuery = false
	NewArrowPart.CanTouch = false

	local Radius = GetRadius(Adornee) + 1.5

	-- Store arrow data
	Arrow.ArrowTable[NewArrowPart] = {
		_Adornee = Adornee, 
		_Point = Point,
		_Radius = Radius,
		_Relative = Relative
	}

	-- Start the single update loop if not already running
	StartUpdateLoop()

	return NewArrowPart
end

Arrow.Remove = function(ArrowPart)
	if Arrow.ArrowTable[ArrowPart] then
		-- Remove from table
		Arrow.ArrowTable[ArrowPart] = nil
		-- Destroy the part
		if ArrowPart then
			ArrowPart:Destroy()
		end
	end
end

Arrow.RemoveAll = function()
	for arrowPart, _ in pairs(Arrow.ArrowTable) do
		-- Destroy the part
		if arrowPart then
			arrowPart:Destroy()
		end
	end
	-- Clear the table
	Arrow.ArrowTable = {}

	-- Disconnect the update loop
	if updateConnection then
		updateConnection:Disconnect()
		updateConnection = nil
	end
end

return Arrow

Simple working module script may not be performant. Comments made by claude. rest of the script by me.

1 Like

you’re cloning the arrow instead of having them in your quiver already thus introducing replication lag. You’re welcome!

1 Like

your codes performacne is great, but i dont like some stuff, and also theres a big change id do to ur code

lets start off light

local function GetRadius(Adornee)
	if not Adornee then return 0 end
	if Adornee.Parent:IsA("Model") then
		local Size = Adornee.Parent:GetExtentsSize()
		return math.max(Size.X, Size.Y, Size.Z)
	elseif Adornee:IsA("BasePart") then
		local Size = Adornee.Size
		return math.max(Size.X, Size.Y, Size.Z)
	end
	warn("No basepart or model")
	return 0
end

unless you are REALLY calling this function for both models and parts, i would just make this function for parts, and adjust all your other code accordingly.

if arrowPart and arrowPart.Parent and data._Adornee and data._Adornee.Parent then

i would remove this conditional completely and make it so that the arrows only cleaned when its ancestry is nilled out, like so

local function bindDestruction(object)
        object.AncestryChanged:Connect(function(_, p)
if (not p) then
Arrow.Remove(arrowPart)
end
-- devforums indentation is non-exidtant so im just not going to indent
end

also instead of doing

if next(Arrow.ArrowTable) == nil then
			if updateConnection then
				updateConnection:Disconnect()
				updateConnection = nil
			end
		end

i woudl do

if (not next(arrow.aarroWTable) and active) then
 updateCOnn:Disconnect()
 active = false
end

because its a lot neater to not rely on the connection as your ‘isActive variable’.

local function UpdateArrow(ArrowPart, Adornee, Point, Radius, Relative)
	if not ArrowPart or not ArrowPart.Parent then return end

	-- If Relative is true, Point is an offset from Adornee, otherwise it's a world position
	local targetPoint = Relative and (Adornee.Position + Point) or Point

	local direction = (targetPoint - Adornee.Position).Unit
	-- Position the arrow at adornee's position, offset by radius in the direction of point
	local arrowPosition = Adornee.Position + (direction * Radius)

	-- Instead of pointing AT targetPoint, create a CFrame that points in the direction
	-- by using a lookVector. We create a point far along the direction vector.
	local lookAtPoint = arrowPosition + (direction * 100) -- 100 studs away in that direction
	ArrowPart.CFrame = CFrame.new(arrowPosition, lookAtPoint) * CFrame.Angles(math.rad(-90), 0, 0)
end

this is good just get rid of that condition especially the arrowPart.Parent bit. the function should never be called if arrowPart is gone.

local NewArrowPart = ArrowPart:Clone()
	NewArrowPart.Parent = workspace
	NewArrowPart.Anchored = true
	NewArrowPart.CanCollide = false
	NewArrowPart.CanQuery = false
	NewArrowPart.CanTouch = false

some obnoxious POS’ on this devfourm dont like :Clone. so use Instance.fromExisting if you want ‘performance’ (im faithful to object:Clone)

theres a few other issues but i’d rather u learn over time
i do want to point out a major change id make

updateConnection = RunService.RenderStepped:Connect(function()
		-- Update all arrows in one loop
		for arrowPart, data in pairs(Arrow.ArrowTable) do
			if arrowPart and arrowPart.Parent and data._Adornee and data._Adornee.Parent then
				UpdateArrow(arrowPart, data._Adornee, data._Point, data._Radius, data._Relative)
			else
				-- Clean up invalid arrows
				Arrow.Remove(arrowPart)
			end
		end
-- ...

well first off use heratbeat, you dont need renderstepped. only use renderstepped for cameras- youre losing performance using it here. two, you could incorporate lerping and deltatime in your code to mediate the speed which the arrow points. if you were to lerp the cframe (which would look better) you would use speed * dt * 60 as the alpha. in other words,

ArrowPart.CFrame = ArrowPart.CFrame:Lerp(CFrame.new(arrowPosition, lookAtPoint) * CFrame.Angles(math.rad(-90), 0, 0), 0.1 * dt * 60)

where dt is from

updateConnection = RunService.Heartbeat:Connect(function(_, dt)

i explain why u do this here Is this line of code valid? - #5 by Ilucere

1 Like

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.