Can this code be improved?

Help and Feedback Code Review
1

So I have a module that manages all the tools in game, and inside that module I have more modules that are the code for the tools. I have it like this because it’s very easy if I want to edit some specific tool.

Tools Manager:

local Tools = {}

local Players = game:GetService("Players")

Players.PlayerAdded:Connect(function(player)
	local character = player.Character or player.CharacterAdded:Wait()
	character.ChildAdded:Connect(function(child)
		if child:IsA("Tool") and not Tools[child] then
			if not script:FindFirstChild(child.Name) then warn("This Tool does not exist!"); return end
			
			local module = require(script:FindFirstChild(child.Name))
			Tools[child] = module.New(child)

			child.Equipped:Connect(function()
				Tools[child]:Equip()
			end)

			child.Unequipped:Connect(function()
				Tools[child]:Unequip()
			end)
			
			child.Activated:Connect(function()
				Tools[child]:Activated()
			end)
		end
	end)
end)

return Tools

And here is one of the tools, which is a Flashlight:

local Tool = {}
Tool.__index = Tool

--// Services
local TweenService = game:GetService("TweenService")

--// Functions
local function TurnOff(model)
	TweenService:Create(
		model.Light.Shadow,
		TweenInfo.new(.15, Enum.EasingStyle.Sine, Enum.EasingDirection.In),
		{Brightness = 0}
	):Play()

	model.Light.Sound2:Play()
end

local function TurnOn(model)
	TweenService:Create(
		model.Light.Shadow,
		TweenInfo.new(.15, Enum.EasingStyle.Sine, Enum.EasingDirection.Out),
		{Brightness = 3}
	):Play()

	model.Light.Sound:Play()
end

function Tool:Equip()
	print("equipped!")
	self.ToolObject.Name = "Flashlight " .. "(" .. self.ToolObject:GetAttribute("Battery") .. "%)"
end

function Tool:Unequip()
	print('unequipped!')
	if self.On then
		self.On = false
		TurnOff(self.ToolObject.Model)
	end
end

function Tool:Activated()
	print('activated!')
	if not self.On and self.ToolObject:GetAttribute("Battery") > 0 then
		self.On = true
		TurnOn(self.ToolObject.Model)

	elseif self.On then
		self.On = false
		TurnOff(self.ToolObject.Model)
	else
		print("no battery")
	end
end

function Tool.New(toolObject)
	local self = {}
	self.ToolObject = toolObject
	self.On = false
	
	print("object created!")
	
	self.ToolObject:GetAttributeChangedSignal("Battery"):Connect(function()
		self.ToolObject.Name = "Flashlight " .. "(" .. self.ToolObject:GetAttribute("Battery") .. "%)"
	end)

	task.spawn(function()
		while true do
			if self.On then
				if self.ToolObject:GetAttribute("Battery") > 0 then
					self.ToolObject:SetAttribute("Battery", self.ToolObject:GetAttribute("Battery") - 1)
					self.ToolObject.Name = "Flashlight " .. "(" .. self.ToolObject:GetAttribute("Battery") .. "%)"
				else
					self.On = false
					TurnOff(self.ToolObject.Model)
				end
			end
			task.wait(.95)
		end
	end)
	
	setmetatable(self, Tool)
	
	return self
end

return Tool

I’m wondering if there’s a way to improve my current code, any help is appreciated!

NOTE: I’m pretty new to metatables and OOP so go easy on me lol

1 Like
2
  1. Use a more efficient way to get the child module

Instead of using script:FindFirstChild(child.Name) every time a tool is added, you can load all child modules of the tool manager module in advance, and then access them directly using the tool name as the key. For example:

local toolModules = {}
for _, toolModuleScript in ipairs(script:GetChildren()) do
    toolModules[toolModuleScript.Name] = require(toolModuleScript)
end
  1. Use auxiliary functions instead of direct manipulation
local function setLightBrightness(model, brightness)
    TweenService:Create(
        model.Light.Shadow,
        TweenInfo.new(.15, Enum.EasingStyle.Sine, Enum.EasingDirection.In),
        {Brightness = brightness}
    ):Play()
end

local function playSound(sound)
    sound:Play()
end

local function TurnOff(model)
    setLightBrightness(model, 0)
    playSound(model.Light.Sound2)
end
3

Sorry for the late reply, and yes i completely agree with loading all modules and then just accessing every module by using the tool name as the key, it makes it a lot easier.

About the tools module I agree because it makes it much cleaner and easier to edit specific parts of the code.

Thanks for your reply!

1 Like