Script for detecting characters on input

In short I have a LocalScript that on MouseButton1 checks if there are any players in a hitbox (a part).

I’ve spent quite some time trying to get this code to this point and for not leaving anything out I ask you if anything can be improved.

Here’s the current state (updated once):

local userInput = game:GetService('UserInputService')

local character = script.Parent

local combatHitbox = character:WaitForChild('CombatResources'):WaitForChild('Hitbox')

userInput.InputBegan:Connect(function(inputObject, gameProcessedEvent)
	if not gameProcessedEvent and inputObject.UserInputType == Enum.UserInputType.MouseButton1 then
		local charactersInHitbox = {}
		local partsInHitbox = workspace:GetPartsInPart(combatHitbox)
		for _, part in ipairs(partsInHitbox) do
			if part.Parent:FindFirstChild('Humanoid') and part.Parent.Name ~= character.Name and character:FindFirstChild('Humanoid').Health > 0 then
				local characterInHitbox = part.Parent
				if not table.find(charactersInHitbox, characterInHitbox) then
					table.insert(charactersInHitbox, characterInHitbox)
				end
			end
		end
		print(charactersInHitbox)
	end
end)
1 Like

What’s the reason for using ipairs?

Because the table is not a hash map, but a list.

ipairs is used on lists and doesn’t work on hash maps.
If you were to try ipairs on a table that has key, value pairs (basically is a hash map/dictionary) it would not work.

As for pairs it is used on tables that have key, value pairs, but they do also work for tables that are lists.

1 Like

There’s no need for the :WaitForChild() in this (you already checked if “Humanoid” exists with the if statement above):

if not table.find(humanoidsInHitbox, part.Parent:WaitForChild('Humanoid')) then
	table.insert(humanoidsInHitbox, part.Parent:WaitForChild('Humanoid'))
end

I never use :WaitForChild() in places where something needs to happen fast but in this case it doesn’t really matter

Also you can store Characters directly and it won’t be any different than storing Humanoids (that is because you’re storing a reference/pointer to the object and not the object itself in the table) and if you ever need for example the HumanoidRootPart it’s going to be more readable if you do character:FindFirstChild("HumanoidRootPart") rather than humanoid.Parent:FindFirstChild("HumanoidRootPart")

2 Likes

Thanks, those tips may not be major, but it definitely makes my code better.

1 Like

You do not need ipairs to loop through partsInHitbox. You could also clean up your code by using overlapparams, to filter out anything that isn’t a humanoid.

otherwise, your code is pretty well written. Nice Job! :smile:

2 Likes

I do not understand what you mean by “You do not need ipairs to loop through partsInHitbox”. If you think pairs is more efficient in this case then you are wrong and should read my reply to the same mistake by scrolling up a bit.

As for OverlapParams I’m not sure if I can pre-filter Humanoids and code the future.