Should I use OOP for this?

Help and Feedback Scripting Support
#1

I have a gun system for a single player game, that is why you will see i trust the client, so please dont bring that up.

Anyways here is the system as of now, for every gun there is a copy of this local script, however some guns that have unique functionality, for example burst and shotguns need the script to be different and as such some parts of the script are modified:

local tool = script.Parent
local player = game.Players.LocalPlayer
local mouse = player:GetMouse()

local UIS = game:GetService("UserInputService")

local shootEvent = game.ReplicatedStorage:WaitForChild("ImportantRemotes"):WaitForChild("DealDamage")
local decreaseAmmo = game.ReplicatedStorage:WaitForChild("ImportantRemotes"):WaitForChild("DecreaseAmmo")
local setAmmo = game.ReplicatedStorage.ImportantRemotes.ReloadGun
local AmmoTextLabel = player.PlayerGui.Gun.Frame.Ammo
local GynTypeTextLabel = player.PlayerGui.Gun.Frame.GunType
--tool.Activated:Connect(function()
--	local MP = mouse.Hit.Position
--	shootEvent:FireServer(player)
--end)
local tool = script.Parent

local BulletsFolder = game.Workspace:WaitForChild("Bullets")

local FastCast = require(game.ReplicatedStorage.FastCastRedux)
-- FastCast.VisualizeCasts = true
local castBehavior = FastCast.newBehavior()

local castParams = RaycastParams.new()
castParams.FilterType = Enum.RaycastFilterType.Exclude
castParams.IgnoreWater = true

local HoldingTrigger = true
local Equipped = false

local shotCooldown = false

local bulletTemplate = Instance.new("Part")
bulletTemplate.Anchored = true
bulletTemplate.CanCollide = false
bulletTemplate.Size = Vector3.new(0.1, 0.1, 1) 
bulletTemplate.Material = Enum.Material.Neon
bulletTemplate.BrickColor = BrickColor.new("Yellow flip/flop")

castBehavior.RaycastParams = castParams
castBehavior.CosmeticBulletContainer = BulletsFolder
castBehavior.CosmeticBulletTemplate = bulletTemplate

local function updateAmmo()
	AmmoTextLabel.Text = script.Parent:GetAttribute("InMag").."/"..script.Parent:GetAttribute("BulletsLeft")
end

local Bullets = {}

local function Hit(cast, result, velocity, bullet)

	game.Debris:AddItem(bullet, 0.01)
	
	Bullets[bullet] = true
	
	local range
	
	if Equipped == false then return end
	local hit = result.Instance
	local character = hit:FindFirstAncestorWhichIsA("Model")
	if character:FindFirstChild("HumanoidRootPart") ~= nil then
		range = (character.HumanoidRootPart.Position - player.Character.HumanoidRootPart.Position).Magnitude
	end
	if character and character:FindFirstChild("Humanoid") then
		shootEvent:FireServer(character.Humanoid, tool, range)
		--character.Humanoid:TakeDamage(20)
	end


end

local function onLengthChanged(cast, lastPoint, direction, length, velocity, bullet)
	--if Equipped == false then return end
	if bullet then
		local bulletLength = bullet.Size.Z / 2
		local offset = CFrame.new(0, 0, -(length - bulletLength))
		bullet.CFrame = CFrame.lookAt(lastPoint, lastPoint + direction):ToWorldSpace(offset)
	end
end

local function equipped()
	Equipped = true
	game.Workspace.Bullets:ClearAllChildren()
	updateAmmo()
	player.PlayerGui.Gun.Enabled = true
	player.PlayerGui.Gun.Frame.GunType.Text = script.Parent.Name
	castParams.FilterDescendantsInstances = {tool.Parent, BulletsFolder}
end


local caster = FastCast.new()

local function shoot()
	if Equipped == false then return end
	if script.Parent:GetAttribute("InMag") > 0 and script.Parent:GetAttribute("CanShoot") == true and player.Character.Humanoid.Health > 0 then
		if script.Parent:GetAttribute('FireType') == "Semi" then
			if shotCooldown then return end
			shotCooldown = true
			decreaseAmmo:FireServer(tool)
			local firePoint = script.Parent.Barrel.FirePoint
			local origin = firePoint.WorldPosition
			local direction = player.Character.HumanoidRootPart.CFrame.LookVector.Unit
			tool.ShootSound:Play()
			caster:Fire(origin, direction * 100, 100, castBehavior)
			task.wait(tool:GetAttribute("FireRate"))
			shotCooldown = false
		elseif script.Parent:GetAttribute('FireType') == "Auto" then
			while task.wait(script.Parent:GetAttribute('FireRate')) do
				if HoldingTrigger == false or Equipped == false or tool:GetAttribute("InMag") <= 0 or tool:GetAttribute("CanShoot") == false then break end
				decreaseAmmo:FireServer(tool)
				local firePoint = script.Parent.Barrel.FirePoint
				local origin = firePoint.WorldPosition
				local direction = player.Character.HumanoidRootPart.CFrame.LookVector.Unit
				tool.ShootSound:Play()

				caster:Fire(origin, direction * 100, 100, castBehavior)

			end
			if  tool:GetAttribute("InMag") == 0 then
				tool.GunEmpty:Play()
			end
		end

	else
		tool.GunEmpty:Play()
	end
end

UIS.InputBegan:Connect(function(input, gp)
	if gp then return end

	if input.KeyCode == Enum.KeyCode.R then
		if Equipped == false then return end
		if tool:GetAttribute("InMag") ~= tool:GetAttribute("MagSize") then
			--tool.Reload:Play()
			player.PlayerGui.Gun.Frame.Ammo.Text = "Reloading..."
			setAmmo:FireServer(script.Parent)
			task.wait(0.1)
			player.PlayerGui.Gun.Frame.Ammo.Text = "Reloading..."
		end

	end

end)

mouse.Button1Down:Connect(function()
	if Equipped == false then return end
	HoldingTrigger = true
end)

mouse.Button1Up:Connect(function()
	if Equipped == false then return end
	HoldingTrigger = false
end)

tool.Unequipped:Connect(function()
	if Equipped == false then return end
	Equipped = false
	player.PlayerGui.Gun.Enabled = false
	game.Workspace.Bullets:ClearAllChildren()
end)

script.Parent:GetAttributeChangedSignal("InMag"):Connect(function()
	if not Equipped then return end
	if tool:GetAttribute("CanShoot") == false then return end
	updateAmmo()
end)

script.Parent:GetAttributeChangedSignal("BulletsLeft"):Connect(function()
	if not Equipped then return end
	if tool:GetAttribute("CanShoot") == false then return end
	updateAmmo()
end)

tool.Equipped:Connect(equipped)
tool.Activated:Connect(shoot)
caster.LengthChanged:Connect(onLengthChanged)
caster.RayHit:Connect(Hit)

this uses fire cast and displays the projectiles on the client as when i did it on the server it was delayed.

This client script also copy and pasted for each gun just plays the hold animation:

local player = game.Players.LocalPlayer
local char = player.Character or player.CharacterAdded:Wait()
local animator = char.Humanoid.Animator
local holdAnimation = game.ReplicatedStorage:WaitForChild('Animations'):WaitForChild("PlayerHoldGun")

local track = animator:LoadAnimation(holdAnimation)

script.Parent.Equipped:Connect(function()
	track:Play()
end)

script.Parent.Unequipped:Connect(function()
	track:Stop()
end)

This server script is only one of deals the appropriate damages to the npc that was hit:

local damageEvent = game.ReplicatedStorage:WaitForChild("ImportantRemotes"):WaitForChild("DealDamage")
local decreaseAmmoEvent = game.ReplicatedStorage:WaitForChild("ImportantRemotes"):WaitForChild("DecreaseAmmo")


local toolDamages = {
	["SMG"] = {
		["15"] = {1,15},
		["10"] = {15,100000}
	},
	["Revolver"] = {
		["75"] = {1,10},
		["50"] = {11,1000}
	},
	["Shotgun"] = {
		["25"] = {1,8},
		["15"] = {9,15},
		["5"] = {15, 100000}
	}
}

damageEvent.OnServerEvent:Connect(function(player, target, gun, range)
	
	if gun:GetAttribute("BulletsLeft") <= 0 then return end
	
	if not toolDamages[gun.Name] then 
		warn("gun is not a entry in toolDamages table")
		return
	end
	
	for i, v in pairs(toolDamages[gun.Name]) do
		if range >= v[1] and range <= v[2] then
			target:TakeDamage(tonumber(i))
			return
		end
	end
end)

decreaseAmmoEvent.OnServerEvent:Connect(function(player, gun)
	if gun:GetAttribute("InMag") <= 0 then return end
	gun:SetAttribute("InMag", gun:GetAttribute("InMag") - 1)
end)

And this server script, also only one of handles reloading:

local event = game.ReplicatedStorage.ImportantRemotes.ReloadGun

local activeReloads = {}

function reloadEvent(player, Gun)
	if player.Character.Humanoid.Health <= 0 then return end
	if Gun:GetAttribute("CanShoot") == false then return end
	
	local switchedGunConnection
	local swappedMidReload = false
	
	Gun.Reload:Play()
	
	switchedGunConnection = Gun.Unequipped:Connect(function()
		swappedMidReload = true
		Gun.Reload:Stop()
	end)
	
	print('Reload event fired with ', Gun.Name)
	
	activeReloads[Gun] = true


	print("Reload started")
	Gun:SetAttribute("CanShoot", false)

	local bulletsLeft = Gun:GetAttribute("BulletsLeft")
	local inMag = Gun:GetAttribute("InMag")
	local maxMagSize = Gun:GetAttribute("MagSize")
	local ammoToFill = maxMagSize - inMag
	
	
	if bulletsLeft == 0 then
		Gun:SetAttribute("CanShoot", true)
		return
	end
	
	task.wait(Gun:GetAttribute("ReloadTime"))
	
	if swappedMidReload == false then
		if bulletsLeft >= ammoToFill then
			Gun:SetAttribute("InMag", maxMagSize)
			Gun:SetAttribute("BulletsLeft", bulletsLeft - ammoToFill)
		else
			Gun:SetAttribute("InMag", bulletsLeft)
			Gun:SetAttribute("BulletsLeft", 0)
		end	
	end
	

	Gun:SetAttribute("CanShoot", true)
	print("Reload ended")
	activeReloads[Gun] = false

end


event.OnServerEvent:Connect(function(player, Gun)
	
	local bulletsLeft = Gun:GetAttribute("BulletsLeft")
	local inMag = Gun:GetAttribute("InMag")
	local maxMagSize = Gun:GetAttribute("MagSize")
	local ammoToFill = maxMagSize - inMag
	
	if ammoToFill == 0 then return end
	
	print(ammoToFill)
	
	if not activeReloads[Gun] then
		player.PlayerGui.Gun.Frame.Ammo.Text = "Reloading..."
		task.spawn(reloadEvent, player, Gun)
		
	end
end)

Currently the system works as intended, it is easy to create a new gun, just copy and paste the local script and the required sounds. The only 2 things that I find annoying is that if I want to make a change to one of the local script for the guns I have to manually change it for every single gun, also the attributes that each gun have to be created manually under as for some reason you cant copy and paste attributes.

I would like to know if these 2 things are reason enough to switch to Object Orientated Programming for the guns. As I see that other people are doing it, I have no experience with it and would like to know if learning and then using it will be worth it or if Iā€™m better off with what I currently have.

#2

You are already coping and pasting code, which is bad. You should change your approach before it all gets out of control.

OOP has its cons. It requires a specific approach to the game design. While it is always beneficial to learn it, it will take a while before you can use it comfortably.

Alternative to OOP is something called Composition Coding, which I use. Rather than creating object GUN and all its various descendants, I prefer to create a specific gun and add common features to it.

First of all I would suggest to try to put all your shared code into a ModuleScript and require it for every gun. Then create a GUN Model template with all attributes already set. It may be enough for your problems.

But if not, then by all means do try to learn OOP and maybe CC. It is better than having a separate script for each gun.

1 Like
#3

What about the ray casts and user input service. Would I have to require the module every time for that

1 Like
#4

You can set up casts and input service inside the module, unless that part of the code is different for each gun. Hard to say without deep knowledge of your system/intentions.

Abstracting the code is a LOT of work, but it will all be worth it in the end, if you plan to keep working on your project and add new guns in the future.