Need some feedback on this gun module I made

Help and Feedback Code Review
#1

So I made a gun module, or more accurately a weapons module which works, but I think that the code could be better.

Here’s the code:

local Gun = {}
Gun.__index = Gun

local FastCast = require(script.Parent:WaitForChild("FastCastRedux"))

function Gun.new()
	
	local newGun = {}
	setmetatable(newGun, Gun)
	
	return newGun
	
end

function Gun:Equip(char, location)
	
	local Torso = char:FindFirstChild("Torso")
	
	Torso.ToolGrip.Part0 = Torso
	Torso.ToolGrip.Part1 = location
	
	return nil
	
end

function Gun:Unequip(char)
	
	local Torso = char:FindFirstChild("Torso")
	Torso.ToolGrip.Part1 = nil
	
	return nil
end

function Gun:Shoot(fireOrigin ,fireDirection, teamKill, gunBullet, spreadAngle, bulletSpeed, bulletGravity, maxDist, fireDelay, container, moreBullets, canDamage)
	
	self.fireOrigin = fireOrigin 
	self.fireDirection = fireDirection 
	self.gunBullet = gunBullet or nil
	self.spreadAngle = spreadAngle 
	self.bulletSpeed = bulletSpeed 
	self.bulletGravity = bulletGravity 
	self.container = container or nil
	self.moreBullets = moreBullets or nil
	
	local RNG = Random.new()
	local TPI = math.pi * 2
	local minSpreadAngle = 0
	
	local tool = script.Parent.Parent
	
	local shot = false
	local Caster = FastCast.new()
	
	local function fireGun(direction)
		
		local directionCF = CFrame.new(Vector3.new(), direction)
		local direction = (directionCF * CFrame.fromOrientation(0, 0, RNG:NextNumber(0, TPI)) * CFrame.fromOrientation(math.rad(RNG:NextNumber(minSpreadAngle, spreadAngle)), 0, 0)).LookVector
		
		local HRP = tool.Parent:FindFirstChild("HumanoidRootPart")
		local movementSpeed = HRP.Velocity
		local modifiedBulletSpeed = (direction * bulletSpeed) + movementSpeed
		
		if container then
			if moreBullets then
				
				local randomX = math.random(-1000, 1000)
				local randomY = math.random(-1000, 1000)
				
				for i = 0, 1, 1 do
					wait()
					local bullet = gunBullet:Clone()
					bullet.CFrame = CFrame.new(fireOrigin, fireOrigin + direction + Vector3.new(randomX, randomY, 0))
			
					bullet.Parent = container
					
					local ignoreList = {container, HRP.Parent}
			
					Caster:FireWithBlacklist(fireOrigin, direction * maxDist, modifiedBulletSpeed, ignoreList ,bullet, false, bulletGravity )
				end
			else
				local bullet = gunBullet:Clone()
				bullet.CFrame = CFrame.new(fireOrigin, fireOrigin + direction)
			
				bullet.Parent = container
				local ignoreList = {container, HRP.Parent}
			
				Caster:FireWithBlacklist(fireOrigin, direction * maxDist, modifiedBulletSpeed, ignoreList ,bullet, false, bulletGravity )
				
			end
		else
			
			local bullet = gunBullet:Clone()
			bullet.CFrame = CFrame.new(fireOrigin, fireOrigin + direction)
			
			bullet.Parent = workspace
			Caster:Fire(fireOrigin, direction * maxDist, modifiedBulletSpeed, bullet, script.Parent.Parent, false, bulletGravity)
		end
	end
	
	local function onRayHit(hitPart, hitPoint, normal, material, segmentVelocity, cosmeticBulletObject)
		
		cosmeticBulletObject:Destroy()
		
		local char = tool.Parent:FindFirstChild("HumanoidRootPart").Parent
		
		-- Lol this part is bugged as heck bro
		if hitPart and hitPart.Parent and hitPart.Parent ~= char then
			
			local humanoid = hitPart.Parent:FindFirstChild("Humanoid")
			local plr = game:GetService("Players"):GetPlayerFromCharacter(hitPart.Parent)
			
			if humanoid then
				
				if teamKill and canDamage then
					
					humanoid:TakeDamage(25)
					
				elseif not teamKill and canDamage and not plr then
					
					humanoid:TakeDamage(25)
					
				else
					
				end
			end
		end
	end
	
	local function onRayUpdated(castOrigin, segmentOrigin, segmentDirection, length, segmentVelocity, cosmeticBulletObject)
		
		local bulletLength = cosmeticBulletObject.Size.Z  / 2
		local baseCFrame = CFrame.new(segmentOrigin, segmentOrigin + segmentDirection)
		cosmeticBulletObject.CFrame = baseCFrame * CFrame.new(0, 0, -(length - bulletLength))
		
	end
	
	Caster.LengthChanged:Connect(onRayUpdated)
	Caster.RayHit:Connect(onRayHit)
	
	if not shot then
		
		shot = true
		fireGun(fireDirection)
		wait(fireDelay)
		shot = false
		
	end
	
	return nil
end


return Gun
2 Likes
#3

Not sure why it’s fake OOP, script looks like it could work. Maybe you’re referring to how the class structure is setup?

#4

Yes, you are 100% correct.

https://roblox.github.io/lua-style-guide/#metatables

#5

Pretty good example of OOP

local GunSuperClass = {}
GunSuperClass.__index = GunSuperClass


function GunSuperClass.new(Gun, Damage)
	local self = {
		Gun = Gun,
		Damage = Damage,
		Connections = {}-- Events
		-- More
	}
	
	setmetatable(self, GunSuperClass)
	
	Gun.Equipped:Connect(function(...)
		self:Equip(...)
	end)
	
	Gun.Unequipped:Connect(function()
		self:Unequip()
	end)
	
	return
end

function GunSuperClass:Shoot()
	
end

function GunSuperClass:Equip()
	
end

function GunSuperClass:Unequip()
	for _, v in next, self.Connections do
		v:Disconnect()
	end
end

-- More functions

return GunSuperClass
1 Like
#6

A few notes here:

  • What the OP was doing wasn’t “fake” OOP, their snippet was adhering to prototypal inheritance in the same way you’ve provided.
  • Naming a class “SuperClass” is incorrect in this instance. Superclasses refer to a class that some given class extends. For instance, a plausible superclass of “Snake” is “Animal”. By no means would you call “Animal” “SnakeSuperClass”.
  • Luau is much different than Lua in terms of performance quirks and optimizations. In Luau, next, table is considered bad style as it is actually slower and less readable than calling ipairs or pairs.
  • A possibly pedantic pointer here is that your GunSuperClass.new function returns nil (through the absence of returning a value at all) which I imagine is just a typo of sorts.
2 Likes