Review my gun system V2

Help and Feedback Code Review
#1 
--GUN OBJECT

local ContextActionService = game:GetService("ContextActionService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
--On ReplicatedStorage
local Contents = ReplicatedStorage.Contents
local Events = ReplicatedStorage.Events
local Package = ReplicatedStorage.Package
--On Contents
local Enums = require(Contents.Enums.Enums)
--On Events
local gunEvent = Events.GunEvent

local Guns = Package.Guns

--Function

local function getInstanceByShootRayCast(self: Gun, mousePosition: Vector3): Instance
	local shootPart = self.tool.Shoot:: BasePart
	local direction = (mousePosition - shootPart.Position).Unit*300
	local result = workspace:Raycast(shootPart.Position, direction)

	if result then
		return result.Instance
	end
end

local function takeDamage(instance: Instance, damage: number)
	local humanoid = instance.Parent:FindFirstChild("Humanoid"):: Humanoid
	if humanoid then
		humanoid:TakeDamage(damage)
	end
end

--Abstract Class
local Gun = {}
Gun.__index = Gun

function Gun.new(player, name, damage, ammunition, reloadTime, shootTime): number
	local self = setmetatable({}, Gun)

	self.name = script.Name
	self.tool = Guns:FindFirstChild(script.Name)
	self.damage = damage
	self.ammunition = ammunition
	self.max_ammunition = ammunition
	self.player = player
	self.shootTime = shootTime
	self.reloadTime = reloadTime
	self.shootState = true
	self.reloadState = false
	self.equippedState = false
	
	return self
end

function Gun:onEquipped()
	if self:isEquipped() then return end
	
	self.equippedState = true
	
	gunEvent:FireClient(self.player, Enums.GunEvent.Equipped, self.name, self.ammunition, self.max_ammunition)
end

function Gun:onUnequipped()
	if not self:isEquipped() then return end
	
	self.equippedState = false
	
	gunEvent:FireClient(self.player, Enums.GunEvent.Unequipped)
end

function Gun:shoot(mousePosition: Vector3) -- PRESS MOUSEBUTTON1CLICK
	if self:isShoot() then return end
	
	if mousePosition and self.ammunition > 0 then
		local instance = getInstanceByShootRayCast(self, mousePosition)
		
		self.shootState = false
		self.ammunition -= 1
		
		if instance then
			takeDamage(instance, self.damage)
		end

		gunEvent:FireClient(self.player, Enums.GunEvent.Shoot, self.ammunition, self.max_ammunition)
		gunEvent:FireClient(self.player, Enums.GunEvent.Animation)
		
		task.wait(self.shootTime)
		
		self.shootState = true
	end
	
	print(self.ammunition)
end

function Gun:reload() --PRESS R
	if self:isReload() then return end
	
	if self.ammunition < self.max_ammunition then
		self.reloadState = true
		print("In Reloading...")
		task.wait(self.reloadTime)
		print("Reload !")
		self.ammunition = self.max_ammunition
		self.reloadState = false
		
		gunEvent:FireClient(self.player, Enums.GunEvent.Reload, self.ammunition, self.max_ammunition)
	else -- TEMPORAIRE !!!
		print("You cant reload !")
	end
end

function Gun:isEquipped(): boolean
	return self.equippedState
end

function Gun:isReload(): boolean
	if not self:isEquipped() then return end
	
	return self.reloadState
end

function Gun:isShoot(): boolean
	if not self:isReload() then return end
	
	return self.shootState
end

export type self = typeof(Gun.new(...))

return Gun

--Guns Info
local Guns = {
	Pistolet = {
		name = "Pistolet",
		damage = 15,
		ammunition = 24,
		reloadTime = 2.5,
		shootTime = 0.25
	}
}

return Guns
#2

I honestly really have nothing to say. (Besides small things)

Your code ticks off my main boxes.

It’s performant.
It’s easy to read. (For the most part. There are some formatting discrepancies.)

I’m not sure why you made getInstanceByShootRayCast a function and not a method of the Gun Abstract class, considering it’s being given self as a parameter.

Maybe changing function names to be slightly more readable, like

isShoot() → canShoot()
isReload() → isReloading()

1 Like