Is this code for a camera module good?

Help and Feedback Code Review
, , ,
#1

I want to release this module i made. it’s all client sided. basically, you bind the player’s camera to a part and it updates via renderstapped (or heartbeat if you set performance to true)
it seems like there are no memory leaks since both bind and unbind are cleaning up the connections properly.

code:

--!native
--!native

local CameraModule = {}
local CurrentCamera = workspace.CurrentCamera
local plr = game.Players.LocalPlayer
local RunService = game:GetService("RunService")
local Connection


if not RunService:IsClient() then
	error("Module must be required on client.", 2)
end

local function Bind(target, performance)

	CurrentCamera.CameraType = Enum.CameraType.Scriptable


	if performance == false then
		local Connection = RunService.RenderStepped:Connect(function()
			CurrentCamera.CFrame = target.CFrame
		end)

	else

		local Connection = RunService.Heartbeat:Connect(function()
			CurrentCamera.CFrame = target.CFrame
		end)
	end
end

function CameraModule:BindCamera(Target: Part, Performance: boolean?)

	if not Target:IsA("Part") then error(`BindCamera expects target to be a Part, got {tostring(Target)}`, 2) end
	if typeof(Performance) ~= "boolean" then Performance = false end

	if Connection then
		Connection:Disconnect()
		Connection = nil
	end

	Bind(Target, Performance)
end

function CameraModule:Unbind()
	if Connection then
		Connection:Disconnect()
		Connection = nil
	end

	CurrentCamera.CameraSubject = plr.Character.Humanoid
	CurrentCamera.CFrame = plr.Character.Head.CFrame
	CurrentCamera.CameraType = Enum.CameraType.Custom
end

return CameraModule
1 Like
#2

Why utilize a table and then index it with the player’s name when this is going to be a client sided script? Since you’re not going to be actively putting connections of other player’s into this module, it doesn’t really make sense and I’d probably recommend just replacing the ActiveConnections and CameraBinds tables with a single variable or something akin to that nature. Over complication for no real benefit.

Outside of that, some code recommendations.

In your code some of your variables are PascalCase and some are just pure abbreviations. I’d recommend removing all abbreviations as “rs” could mean ReplicatedStorage or RunService. It’s easier and clearer if everything is named out.

In a similar vain you use

if not rs:IsClient() then
    error("Module must be required on client.", 2)
end

-- later
if performance == false then

Please don’t use == false lol, just use not performance.

Finally, I’m not sure if you should be using native code generation when you’re not really doing much math. As stated in the original post

You could probably not utilize native code generation for this.

2 Likes
#3

i wanted it to be used able to be used on the server too at one point and just didn’t remove those lol

i was lazy lol, i’ll change it

i used to do that but i fell out of that habit lol

i don’t know what native code gen really helps with lol it really helped with for loops

1 Like
#4

updated my code

--!native

local CameraModule = {}
local CurrentCamera = workspace.CurrentCamera
local plr = game.Players.LocalPlayer
local RunService = game:GetService("RunService")
local Connection


if not RunService:IsClient() then
	error("Module must be required on client.", 2)
end

local function Bind(target, performance)

	CurrentCamera.CameraType = Enum.CameraType.Scriptable


	if performance == false then
		local Connection = RunService.RenderStepped:Connect(function()
			CurrentCamera.CFrame = target.CFrame
		end)

	else

		local Connection = RunService.Heartbeat:Connect(function()
			CurrentCamera.CFrame = target.CFrame
		end)
	end
end

function CameraModule:BindCamera(Target: Part, Performance: boolean?)

	if not Target:IsA("Part") then error(`BindCamera expects target to be a Part, got {tostring(Target)}`, 2) end
	if typeof(Performance) ~= "boolean" then Performance = false end

	if Connection then
		Connection:Disconnect()
		Connection = nil
	end

	Bind(Target, Performance)
end

function CameraModule:Unbind()
	if Connection then
		Connection:Disconnect()
		Connection = nil
	end

	CurrentCamera.CameraSubject = plr.Character.Humanoid
	CurrentCamera.CFrame = plr.Character.Head.CFrame
	CurrentCamera.CameraType = Enum.CameraType.Custom
end

return CameraModule
1 Like
#5

I think this module is too insignificant for you to add it to #resources:community-resources (if you plan to).

Also, there is already something like this: CameraService - A New Camera for a New Roblox

1 Like
#6

I already have, and frankly I don’t really care about that since I mostly share modules I’ve made, use them if you want or not I don’t care

1 Like
#7

On the page About the Community Resources category :

1 Like
#8

Well then flag it to get a mod to remove it to see if it actually violates that rule

1 Like
#9

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