Suggestions to improve tool tracking

Help and Feedback Code Review
#1

I was just looking back at some older code in my game and I came across this code for a badge that works, but I think could be improved. It monitors all tools that a player has and checks whether or not it’s one of the 12 periastron gears. Once a player reaches all 12 of them, it awards the player a badge.

I think it’s messy, and believe there is a better way to set this up. All questions and suggestions are welcome.

local function GivePeriastronsBadge(player) -- This function is called whenever a player joins
	local PeriastronCache = {}
	local ToolConnections = {}
	local Backpack
	local Character
	local function VerifyPeriastron(periastronInstance) -- This is pcalled for each Periastron to verify they all still exist before giving the badge
		if typeof(periastronInstance) == "Instance" and periastronInstance:IsA("Tool") and periastronInstance.Parent == Backpack or periastronInstance.Parent == Character then
			--Still exists	
		else
			error("Periastron no longer exists")
		end
	end
	local function CountPeriastrons(newPeriastron)
		if PeriastronCache[newPeriastron.Name] then return end -- If we've already counted it
		PeriastronCache[newPeriastron.Name] = newPeriastron
		local Count = 0
		for periastronName, periastronInstance in pairs(PeriastronCache) do
			if typeof(periastronInstance) == "Instance" and periastronInstance:IsA("Tool") then
				Count = Count + 1
			end
		end
		if Count == 12 then -- If all values are true, see if they still have them all
			local Success, Err = pcall(function()
				for _, periastron in pairs(PeriastronCache) do
					VerifyPeriastron(periastron) -- Will error if the user doesn't have
				end
			end)
			if Success then -- They indeed have all 12, give the badge
				if not BadgeService:UserHasBadgeAsync(player.UserId, AllPeriastronsId) then
					BadgeService:AwardBadge(player.UserId, AllPeriastronsId)
				end
			else
				PeriastronCache = {}
			end
		end
		local ParentConnection; ParentConnection = newPeriastron:GetPropertyChangedSignal("Parent"):Connect(function() -- Handle parent changes
			if newPeriastron and Character then
				if newPeriastron.Parent and newPeriastron.Parent == Backpack or newPeriastron.Parent == Character then
					return
				end
			end
			PeriastronCache[newPeriastron.Name] = nil
			ToolConnections[newPeriastron.Name] = nil
			ParentConnection:Disconnect()
		end)
		ToolConnections[newPeriastron.Name] = ParentConnection
	end
	player.CharacterAdded:Connect(function(character) -- Handle additions in the character
		Character = character
		Backpack = player:WaitForChild("Backpack")
		character.ChildAdded:Connect(function(child)
			if child:IsA("Tool") then
				if PeriastronsTable[child.Name] then -- This is a separate master table with all of the names to make sure it's not a different tool (not a periastron)
					CountPeriastrons(child)
				end
			end
		end)
		Backpack.ChildAdded:Connect(function(child) -- Handle backpack additions
			if child:IsA("Tool") then
				if PeriastronsTable[child.Name] then -- Same as above
					CountPeriastrons(child)
				end
			end
		end)
		local Humanoid = character:WaitForChild("Humanoid", 3)
		if Humanoid then
			Humanoid.Died:Connect(function() -- Clear all of the values since they'll lose them all if they die
				PeriastronCache = {} -- Empty their cache
				for name, signal in pairs(ToolConnections) do -- Disconnect tool parent changed connections
					if signal then
						signal:Disconnect()
					end
				end
				ToolConnections = {}
			end)
		end
	end)
end

If you’d like a bare place file with the script, here one is: PeriastronBadgeCodeReview.rbxl (19.0 KB)

3 Likes