Gun system help re-organizing

Help and Feedback Code Review
#1 
--!strict


-- Player and currentTool
local player = game.Players.LocalPlayer
local character = player.Character

repeat task.wait() until player and character  
assert(player, " still no player")
assert(character, "still no player character")

local camera = workspace.Camera
local mouse = player:GetMouse()
local humnanoid = character.Humanoid 
local root = character.HumanoidRootPart


-- Services
local runService = game:GetService("RunService")
local replicatedStorage = game:GetService("ReplicatedStorage")
local Settings = require(replicatedStorage.ClientModuleScripts.BigClientSettings)
local uis = game:GetService("UserInputService")
local ContextActionService = game:GetService("ContextActionService")
local soundService = game:GetService("SoundService")
local tweenService = game:GetService("TweenService")
local gunService = require(replicatedStorage.ClientModuleScripts.GunService)
local playerControls = require(replicatedStorage.ClientModuleScripts.PlayerControls)
local mobile = require(replicatedStorage.ClientModuleScripts.MobileHandler)
local playsound = require(replicatedStorage.ClientModuleScripts.SoundPlayingService)
local sounds = replicatedStorage.Sounds
local cleaner = require(replicatedStorage.ClientModuleScripts.Janitor)
local Ebug = require(replicatedStorage.ClientModuleScripts.Ebug)
--local testing = require(replicatedStorage.ClientModuleScripts.Testing)
local Shiny = require(replicatedStorage.ClientModuleScripts.ShinyThing)
local Building = require(replicatedStorage.ClientModuleScripts.PlayerBuilding)


local runService = game:GetService("RunService")
local gunService = require(replicatedStorage.ClientModuleScripts.GunService)
local playerControls = require(replicatedStorage.ClientModuleScripts.PlayerControls)





-- viewmodel stuff
local currentTool = nil
local viewmodel
local viewmodels = replicatedStorage.ViewModels

-- Booleans
local canFire = true
local equipped = false
local isAiming = false
local isSprinting = false

local NormalSprintSpeed = Settings.NormalSprintSpeed
local SprintingSpeed = Settings.SprintingSpeed
local SprintingMax = Settings.SprintingMax

-- remotes
local MeleeRemote = replicatedStorage.RemoteStuff.Melee
local replicateShot = replicatedStorage.RemoteStuff.ShotReplicate
local replicateTween = replicatedStorage.RemoteStuff.replicateTween
local replicateDeath = replicatedStorage.RemoteStuff.NPCDied
local replicatePOS = replicatedStorage.RemoteStuff.UpdatePOS
local stopTweening = false	

local SprintingTime = SprintingMax 
local SprintingCurrentState -- need this for script scope 

-- for meele
local CurrentCooldown = 0 






-- Swaying for viewmodel
local swayCF = CFrame.new()
local swayX
local swayY
local mouseDelta

-- CFs
local aimCF = CFrame.new()
local sprintCF = CFrame.new()

-- anim
local idleAnim 

-- keys
local buildkey = Enum.KeyCode.C
local destroykey = Enum.KeyCode.X


-- idk anymre
local CurrentBuilding = replicatedStorage.BuildingObjectDisplay.Clones["barbed wire fence"]

local function FindAndSetTool(ToSet: any): any
	for i, part in pairs(character:GetChildren()) do
		if part:IsA("Tool") then
			return part
		end
	end
	
	warn("No tool found, currenttool is nothing")
	return nil
end

local function FindInSettings(NeedToSearch: { [string]: number}, WantToFind: string): any
	local FoundValue = nil


	for item, value in pairs(NeedToSearch) do
		if item == WantToFind then
			FoundValue = value
			return FoundValue
		end


	end

	if not FoundValue then
		warn("could not find any value matching")
	end
	
	return nil
end


function createViewmodel()
	if not currentTool then
		warn("There is no current tool, setting the default")
		assert(player,"there is no player")
		player.Backpack:FindFirstChild("RossRifle").Parent = character
		currentTool = FindAndSetTool(currentTool)
	end
	
	viewmodel = viewmodels:FindFirstChild(tostring(currentTool))
	if viewmodel ~= nil then
		viewmodel = viewmodel:Clone()
		viewmodel.Parent = camera
	else
		print("what in the fish no viewmodel")
	end
	for i, what in pairs(character:GetChildren()) do
		if what:IsA("Tool") then
			for e, no in pairs(what:GetChildren()) do
				if no:IsA("UnionOperation") or no:IsA("Part") then
					no.Transparency = 1
				end
			end
		end
	end

	if currentTool:FindFirstChild("Animations") ~= nil then
		idleAnim = currentTool.Animations.Idle
		idleAnim = humnanoid:LoadAnimation(idleAnim)
		idleAnim:Play()

	else
		-- im saying this is uneeded because we arent even using this meth
		warn("no animation object for the currenttool... " .. tostring(currentTool))
	end
end


local function CloneBuildingOnClient(Object: Model, ObjectNewPosition: CFrame): Model
	
	local Clone = Object:Clone() 
	
	Clone.Name = "BUILDING OBJECT HOLOGRAM!!!!"
	Clone.Parent = workspace.ClientStuff
	
	for index, instance in pairs(Clone:GetChildren()) do
		
		if typeof(instance) == "BasePart" then
			instance.CanCollide = false
		end
		
	end
	
	Clone:PivotTo(ObjectNewPosition)
	
	return Clone
end


local function Build(ActionName: string, InputState: Enum.UserInputState, KeyPress: Enum.KeyCode)
	if InputState == Enum.UserInputState.Begin or InputState == Enum.UserInputState.End and KeyPress == buildkey then
		local hit = mouse.Hit :: CFrame
		
		if not hit then
			print(hit)
			return
		end
			
		

			Building:DisplayObject(CurrentBuilding,"HitBoxPart")
			

end
		
	--end
	
	if uis:IsKeyDown(destroykey) then
		
	end
end
	

local function Reload(ActionName: string, InputState: Enum.UserInputState)
	if currentTool.Name ~= "RossRifle" then
		return
	end
	
	
	if InputState ~= Enum.UserInputState.Begin then
		return 
	end
	

	canFire = false
	local reloadAnim = camera:FindFirstChild(tostring(currentTool)).Animations.Reload
	reloadAnim = viewmodel.Humanoid:LoadAnimation(reloadAnim)
	reloadAnim:Play()
	local reloadOutSound = camera:FindFirstChild(tostring(currentTool)).Sounds.MagOut
	reloadOutSound:Play()
	reloadAnim.Stopped:Wait()
	local reloadInSound = camera:FindFirstChild(tostring(currentTool)).Sounds.MagIn
	reloadInSound:Play()
	currentTool.Ammo.Value = FindInSettings(Settings.AmmoCapacity,tostring(currentTool))
	canFire = true
	

end




* local function FireRayCastBullet()
* 		currentTool.Ammo.Value -= 1
* 		canFire = false
* 		local shootAnim = camera:FindFirstChild(tostring(currentTool)).Animations.Shoot
* 		shootAnim = viewmodel.Humanoid:LoadAnimation(shootAnim)
* 		shootAnim:Play()
* 		local shootSound = camera:FindFirstChild(tostring(currentTool)).Sounds.Shoot
* 		shootSound:Play()
* 		local hit, point = gunService.Shoot("e", character.RossRifle.ComingOutFrom.Main.WorldPosition, camera, player)
* 		task.wait(0.5)
* 		canFire = true
* end


local CurrentState = nil
local function Shoot(ActionName: string, InputState: Enum.UserInputState)
	 
	task.spawn(function()  
	if InputState ~= Enum.UserInputState.Begin and InputState ~= Enum.UserInputState.End then
		return
	end	
	CurrentState = InputState
	if FindInSettings(Settings.FullAuto,tostring(currentTool)) == true then
	repeat
	if canFire == true and viewmodel ~= nil and currentTool.Ammo.Value > 0 and InputState ~= Enum.UserInputState.End  then
		
		FireRayCastBullet()
		
	end
	
	until CurrentState == Enum.UserInputState.End or currentTool.Ammo.Value <= 0 or canFire == false 
	
	else 
		
		
	if canFire == true and viewmodel ~= nil and currentTool.Ammo.Value > 0 and InputState ~= Enum.UserInputState.End  then

		FireRayCastBullet()
		
	end
	
	end
	
	end)
end



local function Crouch(ActionName: string, InputState: Enum.UserInputState)

	
	
end 

local sprintBar = player.PlayerGui.Sprint.Background.SprintBar

function Sprint(ActionName: string, InputState: Enum.UserInputState)
	
	task.spawn(function()
	
	if ActionName ~= "Sprint" then
		return 
	end	

	SprintingCurrentState = InputState

	
	
	while SprintingCurrentState == Enum.UserInputState.Begin and SprintingTime > 0 do
		if InputState ~= Enum.UserInputState.Begin or SprintingTime < 0 then
			break
		end
		
		if character:FindFirstChild("Humanoid")  then
		character.Humanoid.WalkSpeed = SprintingSpeed
		else
			character:WaitForChild("Humanoid",5)
			if not character:FindFirstChild("Humanoid") then
				warn("no humanoid for player")
				return
			end
		end
		
		if character.Humanoid.MoveDirection.Magnitude > 0 then
		SprintingTime -= 0.1
		end
		
 		sprintBar:TweenSize(UDim2.new(SprintingTime / SprintingMax, 0, 1, 0), Enum.EasingDirection.In, Enum.EasingStyle.Linear, 0)
		

		if SprintingTime < 0.01 then
			SprintingTime = math.clamp(SprintingTime,0,0.1)
		end

		task.wait(0.01)
	end
	
		character.Humanoid.WalkSpeed = NormalSprintSpeed

	while SprintingCurrentState ~= Enum.UserInputState.Begin and SprintingTime < SprintingMax do
		if InputState ~= Enum.UserInputState.End or SprintingTime > SprintingMax then
			break
		end
		
		if character:FindFirstChild("Humanoid") then
		character.Humanoid.WalkSpeed = NormalSprintSpeed
		else
			character:WaitForChild("Humanoid",5)
			if not character:FindFirstChild("Humanoid") then
				warn("no humanoid for player")
				return
			end
		end
		
		SprintingTime += 0.1
		sprintBar:TweenSize(UDim2.new(SprintingTime / SprintingMax, 0,1, 0), Enum.EasingDirection.In, Enum.EasingStyle.Linear, 0)

		if SprintingTime > SprintingMax then
			SprintingTime = SprintingMax
		end

		task.wait(0.01)
	end
	
	end)
end


local function Stab(ActionName: string, InputState: Enum.UserInputState)
	
	if ActionName ~= "Stab" then
		warn("wrong action name for stab")
		return 
	end
	
	if InputState == Enum.UserInputState.Begin and CurrentCooldown <= 0 then
		
		local params = OverlapParams.new()
		
		
		
		local hitbox = workspace:GetPartBoundsInBox(camera:FindFirstChild(tostring(currentTool)).Blade.CFrame,camera:FindFirstChild(tostring(currentTool)).Blade.Size * 50,params)

		
		MeleeRemote:FireServer(hitbox,currentTool)
				
				
		CurrentCooldown = FindInSettings(Settings.MeleeCooldown,tostring(currentTool))
		
		local stabAnim = camera:FindFirstChild(tostring(currentTool)).Animations.Stab
		stabAnim = viewmodel.Humanoid:LoadAnimation(stabAnim)
		stabAnim:Play()
		
		task.delay(CurrentCooldown,function()
			CurrentCooldown = 0
		end)
		
		
	end
	
end



local function SwitchMeleeOrRangedState()
	ContextActionService:UnbindAction("Build")
	
	
	local AmmoCapacity = FindInSettings(Settings.AmmoCapacity,tostring(currentTool))
	

	if AmmoCapacity >= 0 then		
		print("ranged binded")

		ContextActionService:BindAction("Shoot", Shoot, true, Enum.UserInputType.MouseButton1)
		mobile.SetMobileButtonProperties("Shoot",UDim2.new(0.9,0,0.9,0),tostring(script.Icons["Attack Icon"]))
		
		
		ContextActionService:BindAction("Reload", Reload, true, Enum.KeyCode.R)
		mobile.SetMobileButtonProperties("Reload",UDim2.new(-0.9,0,-0.9,0),tostring(script.Icons["Reloading mobile icon"]))
	else
		print("melee binded")
		ContextActionService:BindAction("Stab", Stab, true, Enum.UserInputType.MouseButton1)
		mobile.SetMobileButtonProperties("Stab",UDim2.new(0.1,0,0.9,0),tostring(script.Icons["Attack Icon"]))
	end
end

* local function ShootingAnims()
* 	if uis:IsKeyDown(Enum.KeyCode.LeftShift) and not uis:IsMouseButtonPressed(Enum.UserInputType.MouseButton1) and viewmodel:FindFirstChild("SprintPart") ~= nil then
* 		local offset = viewmodel.SprintPart.CFrame:ToObjectSpace(viewmodel.HumanoidRootPart.CFrame)
* 		sprintCF = sprintCF:Lerp(offset, 0.15)
* 	else
* 		local offset = CFrame.new()
* 		sprintCF = sprintCF:Lerp(offset, 0.15)
* 	end

* 	if uis:IsMouseButtonPressed(Enum.UserInputType.MouseButton2) then
* 		viewmodel:SetPrimaryPartCFrame(camera.CFrame * swayCF * aimCF)
* 	else
* 		viewmodel:SetPrimaryPartCFrame(camera.CFrame * swayCF * aimCF * sprintCF)
* 	end

* 	if uis:IsMouseButtonPressed(Enum.UserInputType.MouseButton2)  and viewmodel:FindFirstChild("AimPart") ~= nil then
* 		local offset = viewmodel.AimPart.CFrame:ToObjectSpace(viewmodel.HumanoidRootPart.CFrame)
* 		aimCF = aimCF:Lerp(offset, 0.15)
* 	else
* 		local offset = CFrame.new()
* 		aimCF = aimCF:Lerp(offset, 0.15)
* 	end

* end

local function DoStuffWithTool()

	currentTool = FindAndSetTool(currentTool)	
	equipped = true
	createViewmodel()
	if currentTool ~= nil then
		viewmodel = camera:WaitForChild(tostring(currentTool))
		

		SwitchMeleeOrRangedState()

	
	else
		repeat
			warn("no tool found, repeating to find tool until found tool...")
			currentTool = FindAndSetTool(currentTool)			task.wait(0.1)
		until currentTool ~= nil
	end
end


local function NoToolEquipped()
	equipped = false
	viewmodel = camera:FindFirstChild(tostring(currentTool))
	if viewmodel ~= nil then
		viewmodel:Destroy()
	end
	ContextActionService:UnbindAction("Shoot")
	ContextActionService:UnbindAction("Stab")
	ContextActionService:UnbindAction("Reload")
	
	ContextActionService:BindAction("Build",Build,false, buildkey)
end



local function CheckToolRemoved(Given: any): string
	
	if typeof(Given) == "Instance" then
		NoToolEquipped()
	end
	
	return "Good"
end


--character.Destroying:Connect()	
ContextActionService:BindAction("Sprint", Sprint, true, Enum.KeyCode.LeftShift)
character.ChildAdded:Connect(function(given)
	if not given.ClassName then
		return
	end
	
	if given.ClassName == "Tool" then
		DoStuffWithTool()
	end
end)

character.ChildRemoved:Connect(function(given)
	if not given.ClassName then
		return
	end
	
	if given.ClassName == "Tool" then
		CheckToolRemoved(given)
	end
end)

local playerBullet = replicatedStorage.PlayerBullet

* -- This replicates the shot to the client
* local function  replicateShotToClient(plr, point)
* 	if plr ~= player then
* 		local tweenInfo 
* 		local bullet = playerBullet:Clone()
* 		local distance = (bullet.StarterAttach.Position - bullet.EndAttach.Position).Magnitude
* 		bullet.CFrame = plr.Character:FindFirstChildWhichIsA("Tool").ComingOutFrom.Main.WorldCFrame
* 		bullet.StarterAttach.Position = Vector3.zero
* 		bullet.EndAttach.WorldPosition = point
* 		tweenInfo = TweenInfo.new((bullet.StarterAttach.WorldPosition - bullet.EndAttach.WorldPosition).Magnitude / 240, Enum.EasingStyle.Linear, Enum.EasingDirection.Out)
* 		bullet.Parent = workspace
* 		
* 		local tween = tweenService:Create(bullet.StarterAttach, tweenInfo, { Position = bullet.EndAttach.Position })
* 		
* 		local shootSound = Instance.new("Sound", plr.Character:FindFirstChildWhichIsA("Tool"))
* 		shootSound.SoundId = "rbxassetid://230551813"
* 		shootSound:Play()
* 		local shootAnim = plr.Character:FindFirstChildWhichIsA("Tool").Animations.Shoot
* 		shootAnim = plr.Character.Humanoid:LoadAnimation(shootAnim)
* 		shootAnim:Play()
* 		
* 		tween:Play()
* 		tween.Completed:Once(function()
* 			bullet:Destroy()
* 		end)
* 	else
* 		local tweenInfo 
* 		local bullet = playerBullet:Clone()
* 		local distance = (bullet.StarterAttach.Position - bullet.EndAttach.Position).Magnitude
* 		bullet.CFrame = camera:FindFirstChild(tostring(currentTool)).Barrel.Attachment.WorldCFrame
* 		bullet.StarterAttach.Position = Vector3.zero
* 		bullet.EndAttach.WorldPosition = point
* 		tweenInfo = TweenInfo.new((bullet.StarterAttach.WorldPosition - bullet.EndAttach.WorldPosition).Magnitude / 240, Enum.EasingStyle.Linear, Enum.EasingDirection.Out)
* 		bullet.Parent = workspace

* 		local tween = tweenService:Create(bullet.StarterAttach, tweenInfo, { Position = bullet.EndAttach.Position })

* 		local shootSound = Instance.new("Sound", plr.Character:FindFirstChildWhichIsA("Tool"))
* 		shootSound.SoundId = "rbxassetid://230551813"
* 		shootSound:Play()
* 		local shootAnim = plr.Character:FindFirstChildWhichIsA("Tool").Animations.Shoot
* 		shootAnim = plr.Character.Humanoid:LoadAnimation(shootAnim)
* 		shootAnim:Play()

* 		tween:Play()
* 		tween.Completed:Once(function()
* 			bullet:Destroy()
* 		end)
* 	end
* end

* local usedSeeds = {}


* replicateShot.OnClientEvent:Connect(function(plr, point)
* 	replicateShotToClient(plr, point)
* end)


* replicateTween.OnClientEvent:Connect(function(npc, waypoints, seed)
* 	local runAnim = npc.Animations.RunAnim
* 	runAnim = npc.Humanoid:LoadAnimation(runAnim)
* 	runAnim:Play()
* 	for i, waypoint in pairs(waypoints) do
* 		if npc.Humanoid.Health > 0 and table.find(usedSeeds, seed) == nil then 
* 			npc.HumanoidRootPart.CFrame = CFrame.lookAt(npc.HumanoidRootPart.Position, Vector3.new(0,3.25,0) + waypoint)
* 			local Info = TweenInfo.new((npc.HumanoidRootPart.Position - waypoint).Magnitude / 12, Enum.EasingStyle.Linear, Enum.EasingDirection.Out, 0, false, 0)
* 			local tween = tweenService:Create(npc.HumanoidRootPart, Info, {CFrame = CFrame.new(waypoint + Vector3.new(0,3,0)) * npc.HumanoidRootPart.CFrame.Rotation})
* 			tween:Play()
* 			tween.Completed:Wait()
* 			task.wait()
* 			tween:Destroy()
* 		else
* 			print("bruv im dead stop moving mate")
* 		end
* 	end
* 	runAnim:Stop()
* 	for i, possilbeSeed in pairs(usedSeeds) do
* 		if possilbeSeed == seed then
* 			table.remove(usedSeeds, i)
* 			task.wait()
* 			print("oi oi")
* 			replicatePOS:FireServer(npc, nil, "Teleport")	
* 		end
* 	end
* 	print(usedSeeds)
* 	return
* end)

* replicatePOS.OnClientEvent:Connect(function(npc, cFrame: CFrame, typeOfEvent: string, seed)
* 	if typeOfEvent == "Calibrate" then
* 		table.insert(usedSeeds, seed)
* 	elseif typeOfEvent == "Teleport" then
* 		print("da cframe tp")
* 		npc.HumanoidRootPart.CFrame = cFrame
* 	end
* end)



* local function Sway()
* 	viewmodel = camera:FindFirstChild(tostring(currentTool))
* 	viewmodel:SetPrimaryPartCFrame(camera.CFrame)

* 	mouseDelta = uis:GetMouseDelta()/50

* 	swayX = math.clamp(mouseDelta.X, -0.2, 0.2)
* 	swayY = math.clamp(mouseDelta.Y, -0.2, 0.2)

* 	swayCF = swayCF:Lerp(CFrame.new(swayX, swayY, 0), 0.3)
* end


* -- While the tool is equipped (the actual viewmodel)
* runService.RenderStepped:Connect(function()
* 	if character.Humanoid.Health <= 0 then
* 		if camera:FindFirstChild(tostring(currentTool)) ~= nil then
* 			viewmodel:Destroy()
* 		end
* 	end
* 	if equipped == true then
* 		if camera:FindFirstChild(tostring(currentTool)) ~= nil then
* 			Sway()
* 			ShootingAnims()
* 		end
* 	end
* end)




uis.MouseIconEnabled = false

Provide an overview of:

  • What does the code do and what are you not satisfied with?

it does gun system stuff it make viewmodel for weapon and let you shoot and reload and stuff

the code seem very long and it make me have trouble to add more to it

the code i put stars at the left is stuff that my friend added to the script not me

  • What potential improvements have you considered?
    i tried seperating the controls in one module and the functions for the viewmodel in another but then it bugged out because the different modules needed each otehr to work and i didnt know how to fix that

  • How (specifically) do you want to improve the code?
    how do i adjust the funcitons so i can seperate them in different modules without them having to rely on each other?

  • any other improvements because i dont really know what i was doing when i was making this script ;-;
#2

I learnt it the hard way too. What I recommend is to identify clear responsibilities break the script into modules by responsibility not by random function groups for example - a gun controller that handles shooting reloading and ammo an inputhandler, animationmanager soundmanager replicationhandler, to keep it more organized, try to comment on your code too

2 Likes
#3

i might try doing this
the problem i find is i somehow break 50 different things accidently when trying to move them in seperate module scripts so im kind of scared of doing it :moyai: