Feedback on my Hitbox Module

Help and Feedback Code Review
#1

I’m currently working on my first attempt at a personal-use module and I’m looking for some feedback on whether anything could be optimized

It uses a while loop so I already know that may not be the most efficient nor accurate method, but it works for the type of things I’ve used it for so far

Hitbox (Fix 2).rbxm (2.3 KB)

Initializing

local HitboxModule = require(ModuleScriptHere)
local hitboxObject = HitboxModule.new(BasePartHere)

Functions

hitboxObject:GetTouchingCharacters() -- returns table of characters inside hitbox
hitboxObject:GetTouchingPlayers() -- returns table of players inside hitbox

Events

hitboxObject.PlayerEntered      (returns player) -- Fired when player enters
hitboxObject.PlayerExited      (returns player) -- Fired when player exits
hitboxObject.CharacterEntered      (returns character) -- Fired when character enters
hitboxObject.CharacterExited      (returns character) -- Fired when character exits

Example
Here I give the player a forcefield when entering, and remove it when exiting

local HitboxModule = require(game.ReplicatedStorage.Hitbox)
local hitboxObject = HitboxModule.new(workspace.ExamplePart)

hitboxObject.PlayerEntered:Connect(function(player)
	print(player, "entered the box")
	local forceField = player.Character:FindFirstChildOfClass("ForceField")

	if not forceField then
		local forceField = Instance.new("ForceField")
		forceField.Visible = true
		forceField.Parent = player.Character
	end
end)

hitboxObject.PlayerExited:Connect(function(player)
	print(player, "exited the box")
	local forceField = player.Character:FindFirstChildOfClass("ForceField")
	forceField:Destroy()
end)

Module Script

-- CorruptionGhost
local Players = game:GetService("Players")
local Functions = require(script.Functions)

local Hitbox = {}
Hitbox.__index = Hitbox

-- [Constructor]
function Hitbox.new(hitboxPart : BasePart)
	-- Checking BasePart
	if not (hitboxPart:IsA("BasePart")) then
		warn(script:GetFullName() .. ":", hitboxPart, "is not a valid BasePart")
		return nil
	end
	
	-- Creating Object
	local self = {}
	setmetatable(self, Hitbox)
	
	self.Part = hitboxPart
	
	-- Creating Events
	local PlayerEntered = Instance.new("BindableEvent")
	self.PlayerEntered = PlayerEntered.Event
	
	local PlayerExited = Instance.new("BindableEvent")
	self.PlayerExited = PlayerExited.Event
	
	local CharacterEntered = Instance.new("BindableEvent")
	self.CharacterEntered = CharacterEntered.Event
	
	local CharacterExited = Instance.new("BindableEvent")
	self.CharacterExited = CharacterExited.Event
	
	-- Detecting Character Enter/Exit (While Loop Coroutine)
	self.Active = true
	local hitboxCoroutine = coroutine.create(function()
		local player;
		
		local previouslyTouching = self:GetTouchingCharacters()
		local currentlyTouching = {}
		local tableDifference = true
		
		local charactersEntered;
		local charactersExited;
		
		
		while self.Active do
			currentlyTouching = self:GetTouchingCharacters()

			charactersEntered = Functions.ExclusiveInTable(currentlyTouching, previouslyTouching)
			for _, character in pairs(charactersEntered) do
				CharacterEntered:Fire(character)
				
				player = Players:GetPlayerFromCharacter(character)
				if player then
					PlayerEntered:Fire(player)
				end
			end
			
			charactersExited = Functions.ExclusiveInTable(previouslyTouching, currentlyTouching)
			for _, character in pairs(charactersExited) do
				CharacterExited:Fire(character)
				
				player = Players:GetPlayerFromCharacter(character)
				if player then
					PlayerExited:Fire(player)
				end
			end
			
			previouslyTouching = currentlyTouching

			task.wait()
		end
	end)
	
	coroutine.resume(hitboxCoroutine)
	
	return self
end

-- [Object Functions]
function Hitbox:GetTouchingPlayers() -- Get Players in Hitbox
	local touchingPlayers = {}
	local player

	for _, part in pairs(workspace:GetPartsInPart(self.Part)) do
		player = game.Players:GetPlayerFromCharacter(part.Parent)
		if not table.find(touchingPlayers, player) then
			table.insert(touchingPlayers, player)
		end
	end

	return touchingPlayers
end

function Hitbox:GetTouchingCharacters() -- Get Characters in Hitbox
	local touchingCharacters = {}

	for _, part in pairs(workspace:GetPartsInPart(self.Part)) do
		local character = part.Parent
		if not table.find(touchingCharacters, character) and character:FindFirstChildOfClass("Humanoid") then
			table.insert(touchingCharacters, character)
		end
	end

	return touchingCharacters
end

function Hitbox:Destroy() -- Destroy a Hitbox Object
	self.Active = false
	self = nil
end


return Hitbox
1 Like
#2

It is not best practise to declare methods inside the constructor of a class. I recommend to declare them after the constructor instead.

Secondly, you should avoid while loops at all costs if you do not fully depend on them. This can be done by keeping tracks of all parts that the hitbox touches with the Touched and TouchEnded events. Then you can also keep track of whether a new or old part belongs to a player’s character.

#3

Thanks for the feedback, I’ve moved the functions outside of the constructor

As for the second suggestion, I’ve actually attempted that before:

-- Detecting Character Enter/Exit (Touched Event)
	local player;
	
	local previouslyTouching = self:GetTouchingCharacters()
	local currentlyTouching = {}
	
	local charactersEntered;
	local charactersExited;
	
	local function onTouch(hitPart) 
		currentlyTouching = self:GetTouchingCharacters()

		charactersEntered = Functions.ExclusiveInTable(currentlyTouching, previouslyTouching)
		for _, character in pairs(charactersEntered) do
			CharacterEntered:Fire(character)

			player = Players:GetPlayerFromCharacter(character)
			if player then
				PlayerEntered:Fire(player)
			end
		end

		charactersExited = Functions.ExclusiveInTable(previouslyTouching, currentlyTouching)
		for _, character in pairs(charactersExited) do
			CharacterExited:Fire(character)

			player = Players:GetPlayerFromCharacter(character)
			if player then
				PlayerExited:Fire(player)
			end
		end

		previouslyTouching = currentlyTouching
	end 
	
	
	hitboxPart.Touched:Connect(onTouch)
	hitboxPart.TouchEnded:Connect(onTouch)

This code never works reliably, and seems to fire the events at random; I’ve also tried creating other variations of Touched event functions and they all seem to have the same behaviour. I can’t seem to understand why, it’s either a problem with the way I’ve scripted it or the events themselves are unreliable.

1 Like
#4

For some reason, humanoid state changes can cause touches to disconnect for a single frame. The solution I found is to wait() after the touch has ended and only execute the disconnect code if another touch isn’t received in rapid succession.

Module I wrote demonstrating the method. 
export type Trigger = {
	Touched:RBXScriptSignal,
	TouchEnded:RBXScriptSignal
}

local Trigger = {}

function Trigger.new(touchPart:BasePart, partFilter:(BasePart)->(any?)):Trigger
	local touchCount:{[any]:number} = {}
	local touched:BindableEvent = Instance.new("BindableEvent")
	local touchEnded:BindableEvent = Instance.new("BindableEvent")
	
	local self:Trigger = {
		Touched = touched.Event,
		TouchEnded = touchEnded.Event
	}
	
	touchPart.TouchEnded:Connect(function (otherPart:BasePart)
		local hit = if partFilter then partFilter(otherPart) else otherPart
		
		if not hit then return end
		
		-- Give time to twitch
		task.wait()
		
		-- Adjust touch count
		touchCount[hit] -= 1
		
		-- Sink all but last event
		if 0 < touchCount[hit] then return end
		
		-- Remove touch counter
		touchCount[hit] = nil
		
		-- End touch
		touchEnded:Fire(hit)
	end)
	
	touchPart.Touched:Connect(function (otherPart:BasePart)
		local hit = if partFilter then partFilter(otherPart) else otherPart
		
		if not hit then return end
		
		if touchCount[hit] then
			touchCount[hit] += 1
			return
		end
		
		-- Intiate touch counter
		touchCount[hit] = 1
		
		-- Begin touch
		touched:Fire(hit)
	end)
	
	setmetatable(self, Trigger)
	
	return self
end

return Trigger
Example usage. 
-- Add as a LocalScript to StarterCharacterScripts and scatter
-- a few parts around the workspace tagged with "Interactive"

local Trigger = require(game:GetService("ReplicatedStorage"):WaitForChild("Trigger"))

local character = script.Parent
local humanoid = character:WaitForChild("Humanoid")
local rootPart = character:WaitForChild("HumanoidRootPart")

local touchPart = Instance.new("Part", character)
touchPart.BrickColor = BrickColor.Blue()
touchPart.CFrame = rootPart.CFrame
touchPart.Material = Enum.Material.ForceField
touchPart.Shape = Enum.PartType.Ball
touchPart.Size = Vector3.new(7, 7, 7)

local touchWeld = Instance.new("Weld", touchPart)
touchWeld.Part0 = rootPart
touchWeld.Part1 = touchPart

local function partFilter(hit)
	return game:GetService("CollectionService"):HasTag(hit, "Interactive") and hit
end

local trigger = Trigger.new(touchPart, partFilter)

trigger.Touched:Connect(function (hit)
	touchPart.BrickColor = BrickColor.Green()
end)

trigger.TouchEnded:Connect(function (hit)
	touchPart.BrickColor = BrickColor.Blue()
end)

Edit: Fixed an error in the code.