Help me optimize my code please!

[R3ivik]

Hello, so I’m currently building a horror survival shooter game with my friend and I was just coding the FPS framework but its quite a mess. Could anyone help me optimize and organize this for performance, safety and all that jazz

NOTE: I only have the client side done, not the server side yet and no weapon functionality is scripted yet. Only the equip system and that stuff.
+Give recommendations on how I should structure the serverside and each upcoming weapon’s code.

Thanks in advance.

WEAPON SYSTEM CLIENT HANDLER:

local Player = game:GetService("Players").LocalPlayer
local mouse = Player:GetMouse()
local UIS = game:GetService("UserInputService")

local WeaponFramework = game:GetService("ReplicatedStorage"):WaitForChild("WeaponFramework")
local WeaponsFolder = game:GetService("ReplicatedStorage"):WaitForChild("Weapons")
local EquipWeaponEvent = WeaponFramework.Events.EquipWeapon

local WeaponModule = require(WeaponFramework.Modules.WeaponModule)

local VM = WeaponFramework.Viewmodel

local ClickedKeyBind = false
local Equipped = false

local Slot = 0

--Scroll wheel weapons
local SWWeapons = {
	
	[Enum.KeyCode.One] = "FlashLight",
	[Enum.KeyCode.Two] = "Pistol"
	
}

--Keybind weapons
local KBWeapons = {
	
	[Enum.KeyCode.One] = "One",
	[Enum.KeyCode.Two] = "Two"
	
}

local function KeyBindEquip(input, gp)
	if not gp then
		if KBWeapons[input.KeyCode] then
			local ChosenWeapon = SWWeapons[input.KeyCode]			
			local CurrentWeapon = WeaponsFolder[ChosenWeapon]
			
			if CurrentWeapon then
				if ClickedKeyBind == false then
					ClickedKeyBind = true
					
					local ClonedVM = VM:Clone()
					local ClonedWN = CurrentWeapon:Clone()
					
					local WeaponHoldAnimClient = ClonedWN.Animations.HoldingAnimationClient
					
					WeaponModule.EnableViewmodel(ClonedVM)
					WeaponModule.Equip(ClonedWN, ClonedVM, WeaponHoldAnimClient)
					WeaponModule.Weld(ClonedWN)
					
					EquipWeaponEvent:FireServer(ClonedVM)
					
					Equipped = true
					
					UpdaterFunction = game:GetService("RunService").RenderStepped:Connect(function(dt) --This is ugly
						WeaponModule.UpdateViewmodel(ClonedVM, dt)
					end)
				else       --Make the weapon actually go into the players hand, then mess with serverside
					ClickedKeyBind = false
					Equipped = false
					UpdaterFunction:Disconnect()
					WeaponModule.DisableViewmodel()
				end
			end
		end
		
		if input.UserInputType == Enum.UserInputType.MouseButton1 then
			if Equipped == true then
				
			end
		end
	end
end

UIS.InputBegan:Connect(KeyBindEquip)

WEAPON MODULE SCRIPT:

local Weapon = {}

function Weapon.UpdateViewmodel(Viewmodel, DT)
	if Viewmodel and Viewmodel:FindFirstChild("HumanoidRootPart") then
		Viewmodel.HumanoidRootPart.CFrame = game.Workspace.CurrentCamera.CFrame
	end
end

function Weapon.EnableViewmodel(Viewmodel)
	if workspace.Camera:FindFirstChild("Viewmodel") == nil then
		Viewmodel.Parent = game.Workspace.Camera --Must clone the viewmodel in another script, trust me bro
	end
end

function Weapon.DisableViewmodel()
	for i, vm in pairs(workspace.Camera:GetChildren()) do
		if vm.Name == "Viewmodel" then
			vm:Destroy()
		end
	end
end

function Weapon.Weld(WeaponModel)
	if WeaponModel then
		local Handle = WeaponModel:WaitForChild("Handle")

		for i, parts in ipairs(WeaponModel:GetDescendants()) do
			if parts:IsA("BasePart") and parts ~= Handle then
				if parts:FindFirstChildOfClass("Weld") == nil then
					local WMotor = Instance.new("Motor6D")
					WMotor.Name = parts.Name
					WMotor.Part0 = Handle
					WMotor.Part1 = parts
					WMotor.C0 = WMotor.Part0.CFrame:Inverse() * WMotor.Part1.CFrame
					WMotor.Parent = Handle
				else
					parts:FindFirstChildOfClass("Weld"):Destroy()
				end
			end
		end
	end
end

function Weapon.Equip(Weapon, Viewmodel, HoldAnim)
	local Handle = Weapon:FindFirstChild("Handle")
	local HRPH = Viewmodel:WaitForChild("HumanoidRootPart").Handle
	
	Weapon.Parent = Viewmodel
	HRPH.Part1 = Handle
	
	local HoldingAnimation = Viewmodel.AnimationController:LoadAnimation(HoldAnim)
	HoldingAnimation:Play()
end

return Weapon

[Fast_Duck]

Inside scripts use collection service for multiple instances. Use folders everywhere to make the debugging process easier.
Use in if statements return to not have heavily nested code. Use the fast signals module you can find it at “https://create.roblox.com/store/asset/6532460357/FastSignal-A-consistent-Signal-library”.
Do not create to many lambda functions and do not forget to disconnect your connections that you do not use anymore.
Use the official naming conventions you can find them at “Variables | Documentation - Roblox Creator Hub”. Do not put everything in replicated storage because exploiters can access and copy it.
Do not make to much abstraction because at that point it is better to refactor it without the abstraction. Make sections in your code with comments like – Variables. Use actors to have eventually multi threaded code.

[R3ivik]

Thanks for the reply!

Tho I have one more question to ask, I’m experiencing some strange errors so if you could then please look at my other post

link:

> Blockquote