Looking for feedback on my tool class

Help and Feedback Code Review
#1

image
All the tool class does is create a tool and parents it to the character that player selects from another menu.

I create an object of the input class which then has reference to the tool as it creates an object of the tool there.

-- Services
local userInputService = game:GetService("UserInputService")
local replicatedStorage = game:GetService("ReplicatedStorage")

--Folders
local classes = replicatedStorage:WaitForChild("Classes")

--Classes
local maid = classes:WaitForChild("Maid")
local maidClass = require(maid)
local tool = classes:WaitForChild("Tool")
local toolClass = require(tool)

local Input = {}
Input.__index = Input

function Input.New(player, item, slot)
	local self = setmetatable({}, Input)
	print("intialise input")
	
	local maidObject = maidClass.new()
	self._Maid = maidObject
	self._ToolObject = toolClass.New(player, item)
	self._Slot = slot
	
	self._Maid:GiveTask(self._ToolObject)
	
	self:BindActions()
	return self
end

function Input:_BindAction(keyCode, stateName)
	print("Binding action: ".. stateName)
	self._Maid:GiveTask(userInputService.InputBegan:Connect(function(input, gameProcessed)
		if gameProcessed then
			return
		end

		if input.KeyCode ~= keyCode and 
			input.UserInputType ~= keyCode 
		then  
			return
		end
		
		if stateName ~= "Equipped" then
			if not self._ToolObject:GetTool():GetAttribute("Equipped") then
				return
			end
		end
		--print("Input is NOT a game processed event and IS the bound key, continuing...")

		-- when input begins equipped is gonna be false
		self:PlayState(stateName) ---This is where you define your handler function
	end))
	
	
	self._Maid:GiveTask(userInputService.InputEnded:Connect(function(input, gameProcessed)
		if gameProcessed then
			return
		end

		if input.KeyCode == keyCode or 
			input.UserInputType ~= keyCode 
		then  
			return
		end

		if stateName ~= "Equipped" then
			if not self._ToolObject:GetTool():GetAttribute("Equipped") then
				return
			end
		end
		--print("Input is NOT a game processed event and IS the bound key, continuing...")

		-- when input begins equipped is gonna be false
		self:EndState(stateName)  ---This is where you define your handler function
	end))
end

---This function essentially binds all actions at once, this is where I organize my "actions".
function Input:BindActions()
	local enum = nil
	
	if self._Slot == 1 then
		enum = Enum.KeyCode.One
	elseif self._Slot == 2 then
		enum = Enum.KeyCode.Two
	elseif self._Slot == 3 then
		enum = Enum.KeyCode.Three
	end
	
	local states = { ---This is how I map all keys and state names, these are essentially strings that are referenced for my handler function.
		enum, "Equipped", -- don't need this unless I'm making a pseudo tool
		Enum.UserInputType.MouseButton1, "Shoot",
		
	}
	
	for index = 1, #states, 2 do ---This is how I iterate through the table, I am using a for loop because I am lazy lol.
		self:_BindAction(states[index], states[index + 1])
	end
end

function Input:PlayState(stateName) 
	if stateName == "Shoot" then
		self._ToolObject.MouseDown:Fire()
	end
end

function Input:EndState(stateName) 
	if stateName == "Shoot" then
		self._ToolObject.MouseUp:Fire()
	end
end

function Input:Destroy()
	self._Maid:DoCleaning()
end

return Input

Tool class

-- Services
local replicatedStorage = game:GetService("ReplicatedStorage")
local isServer = game:GetService("RunService"):IsServer()
local runService = game:GetService("RunService")

-- Folders
local events = replicatedStorage:WaitForChild("Events")
local classes = replicatedStorage:WaitForChild("Classes")
local functions = replicatedStorage:WaitForChild("Functions")

local components = replicatedStorage:WaitForChild("Components")
local objects = components:WaitForChild("Objects")
local fpmodels = objects:WaitForChild("FPModels")

-- Events
local basicToolEvent = events:WaitForChild("RemoteEvent")

-- Functions
local basicToolFunction = functions:WaitForChild("RemoteFunction")

-- Classes
local maid = classes:WaitForChild("Maid")
local maidClass = require(maid)
local Signal = require(classes.Signal)
local motor = script:FindFirstChild("Motor")
local motorClass = require(motor);
local render = nil
local renderClass = nil
local ViewModel = nil
local ViewModelClass = nil
local animation = nil
local animationClass = nil
if not isServer then
	render = script:FindFirstChild("Render")
	renderClass = require(render)
	ViewModel = script:FindFirstChild("ViewModel") 
	ViewModelClass = require(ViewModel)
	animation = script:FindFirstChild("Animation")
	animationClass = require(animation)
end

local Tool = {}
Tool.__index = Tool	

function Tool.New(player, name)
	local self = setmetatable({}, Tool)
	print("Initialize Tool ", player, name)

	local maidObject = maidClass.new()
	self._Maid = maidObject
	self.Player = player
	if (isServer) then
		self.Tool = Instance.new("Tool")
		self._Name = name
		self._Backpack = self.Player.Backpack
		self.Tool.Name = self._Name
		self.Tool.RequiresHandle = false
		self.Tool.CanBeDropped = false
		self.Tool.Enabled = false	
		self.Tool.Parent = self._Backpack

		self._MotorObject = motorClass.New(player, self.Tool) -- ,script.Configuration -- could call this in constructor?
		self:_InsertModel()
	else
		self.Tool = basicToolFunction:InvokeServer(name)
		self._equipped = false
		self._FinishedUnequipped = true
		self._ToolModel = nil
		
		self._conn1 = nil
		self._conn2 = nil
		
		self.MouseDown = Signal.new()
		self.MouseUp = Signal.new()

		self._ViewModelObject = ViewModelClass.New(self.Tool)
		self._MotorObject = motorClass.New(player, self.Tool, self._ViewModelObject) -- ,script.Configuration
		self._RenderObject = renderClass.New(player, self.Tool, self._ViewModelObject)
		self._AnimationObject = animationClass.New(player, self.Tool, self._ViewModelObject)

		self._Maid:GiveTask(self._MotorObject)
		self._Maid:GiveTask(self._RenderObject)
		self._Maid:GiveTask(self._ViewModelObject)
		self._Maid:GiveTask(self._AnimationObject)

		self:_InsertModel()
		self:_SetViewModelTransparency(false)
		self:_CreateAttributes()

		self._Maid:GiveTask(self.Tool:GetAttributeChangedSignal("FirstPerson"):Connect(function()
			self:_ObserveFirstPerson()
		end))

		self._Maid:GiveTask(self.Tool:GetAttributeChangedSignal("Equipped"):Connect(function()
			self:_ObserveEquipped()
		end))
		
		self._Maid:GiveTask(self.Tool:GetAttributeChangedSignal("FinishedUnequippedAnim"):Connect(function()
			self:_ObserveFinishedUnequippedAnim()
		end))
		
		self._Maid:GiveTask(self.Tool.Equipped:Connect(function()	
			self._conn1 = runService.Heartbeat:Connect(function(dt) 
				-- waiting to equipp tool
				if not self.equipped and self.Tool:GetAttribute("FinishedUnequippedAnim") and self._FinishedUnequipped then 
					if self._conn1 == self._Maid.Falling then
						self._Maid.Falling = nil
						self._conn1:Disconnect()
						self._conn1 = nil
						self:_Equipped()
					end
				end
			end)
			self._Maid.Falling = self._conn1
		end))

	
		self._Maid:GiveTask(self.Tool.Unequipped:Connect(function()
			self._conn2 = runService.Heartbeat:Connect(function(dt) 
				-- waiting to equipp tool
				if self._equipped and not self.Tool:GetAttribute("FinishedUnequippedAnim") then
					if self._conn2 == self._Maid.Falling2 then
						self._Maid.Falling2 = nil
						self._conn2:Disconnect()
						self._conn2 = nil
						self:_Unequipped()
					end
				end
			end)
			self._Maid.Falling2 = self._conn2
		end))
		
		self._Maid:GiveTask(self.MouseDown:Connect(function()
			print("MouseDown signal received ", self.Tool)
			-- play the related animation to input
			-- if it's a gun it needs to shoot right
			-- if it's a hammer it needs to build
			-- so pull the related module to the tool
			-- and fire related functions to input
		end))
		

		self._Maid:GiveTask(self.MouseUp:Connect(function()
			print("MouseUp signal received ", self.Tool)
		end))

	end

	return self
end

function Tool:_CreateAttributes()
	self.Tool:SetAttribute("FirstPerson", false)
	self.Tool:SetAttribute("Equipped", false)
	self.Tool:SetAttribute("FinishedUnequippedAnim", true)
end

function Tool:_ObserveFirstPerson()
	local firstPerson = self.Tool:GetAttribute("FirstPerson")
	self:_SetViewModelTransparency(firstPerson)
	self:_SetToolModelTransparency(firstPerson)
end

function Tool:_ObserveEquipped()
	self._equipped = self.Tool:GetAttribute("Equipped")
	if self._equipped then
		self._AnimationObject:Equipped()
	else
		local tool = self.Player.Character:FindFirstChildOfClass("Tool")
		if not tool then
			self._AnimationObject:Unequipped()
		else
			self._AnimationObject:Switched()
		end
	end
end

function Tool:_ObserveFinishedUnequippedAnim()
	local FinishedUnequippedAnim = self.Tool:GetAttribute("FinishedUnequippedAnim")
	if FinishedUnequippedAnim then
		self._RenderObject:DeRender()
		self.Tool:SetAttribute("FirstPerson", false)
		self._FinishedUnequipped = true
	end
end
		
function Tool:GetTool()
	return self.Tool
end

function Tool:_Equipped()
	self._FinishedUnequipped = false
	self.Tool:SetAttribute("FinishedUnequippedAnim", false)
	self.Tool:SetAttribute("Equipped", true)
	self._RenderObject:Render()
end

function Tool:_Unequipped()
	self.Tool:SetAttribute("Equipped", false)
end

function Tool:_InsertModel()
	local model = fpmodels[self.Tool.Name]
	if model then
		if (isServer) then
			self._ToolModel = model:Clone()
			self._ToolModel.Parent = self.Tool
		else
			local viewModel = self._ViewModelObject:GetViewModel()
			local modelClone = model:Clone()
			modelClone.Parent = viewModel
			self._ToolModel = self.Tool[self.Tool.Name]
		end
		self._MotorObject:AttachMotors()
	end
end

function Tool:_SetViewModelTransparency(bool) 
	local value = if bool then 0 else 1
	local viewModel = self._ViewModelObject:GetViewModel()
	local model = self._ViewModelObject:GetModel()
	local tool = self.Player.Character:FindFirstChildOfClass("Tool")
	
	if tool and self.Tool.Name == tool.Name or not tool then
		viewModel["Right Arm"].LocalTransparencyModifier = value
		viewModel["Left Arm"].LocalTransparencyModifier = value
	end
	
	-- will want to put the ignore list in a table
	for i, v in pairs(model:GetChildren()) do
		if v:IsA("BasePart") and
			v.Name ~= "Barrel" and 
			v.Name ~= "Aim" 
		then
			v.Transparency = value
		end
	end
end

function Tool:_SetToolModelTransparency(bool)
	local value = if bool then 1 else 0
	-- will want to put the ignore list in a table
	for i, v in pairs(self._ToolModel:GetChildren()) do
		if v:IsA("BasePart") and 
			v.Name ~= "Barrel" and
			v.Name ~= "Aim" 
		then
			v.Transparency = value
		end
	end
end

function Tool:Destroy()
	self._Maid:DoCleaning()
	print("clean up tools")
end

return Tool

This is my first using OOP in roblox so I’m not sure how well I’ve structured my code any feedback is welcome. I’m planning on adding much more functionality as all you can do is equip / unequip atm.

2 Likes