Melee Damage - Improvements?

Help and Feedback Code Review
#1

I’m currently trialling different methods of creating a Melee damaging system - Below is my latest attempt.

Currently, it’s only in a single Part moving back and forth to test the reaction rate; ideally, the Heartbeat is going to run whilst the Sword is ‘Swinging’ - That way it’ll be able to detect any Humanoid it collides with and enter their details into a Dictionary.

Thoughts on this?

local Connection

local Part = script.Parent
local _HitHumanoid = {}

local Min_Damage, Max_Damage = 5, 15

--||--||--||--

Part.Touched:Connect(function()	end)

--||--||--||--

Tween = function(Obj, Info, Properties)
	local TweenService = game:GetService("TweenService")
	TweenService:Create(Obj, Info, Properties):Play()
end

--||--||--||--

Enable = function()
	Connection = game:GetService("RunService").Heartbeat:Connect(function()
		local TouchingPart = Part:GetTouchingParts()
		for _,Part in pairs (TouchingPart) do
			local Humanoid = Part.Parent:FindFirstChildOfClass("Humanoid")
			if Humanoid then
				if _HitHumanoid[Humanoid] == nil then
					_HitHumanoid[Humanoid] = true
				end
				Damage()
			end
		end
	end)
end

--||--||--||--

Damage = function()
	for Humanoid, __ in pairs (_HitHumanoid) do
		if _HitHumanoid[Humanoid] == true then
			_HitHumanoid[Humanoid] = false
			Humanoid:TakeDamage(math.random(Min_Damage, Max_Damage))
			Connection:Disconnect()
		end
	end
end

--||--||--||--

while true do
	wait(2); Enable()
	_HitHumanoid = {}
	Tween(Part, TweenInfo.new(1), {Position = Vector3.new(0, 3, 4)})
	wait(2); Enable()
	_HitHumanoid = {}
	Tween(Part, TweenInfo.new(1), {Position = Vector3.new(0, 3, -4)})
end

--||--||--||--
2 Likes
#2

Nice job but do you maybe want to make it so once it hits it’s first humanoid it stops doing damage. This is to stop people just spinning and hitting people multiple times with one swing. Unless thats what you’re going for

1 Like
#3

Hi - Yes, that is ideally what I’m going for.

For every “Strike” it’ll run the Heartbeat and damage each Touching Humanoid once - adding them into a dictionary to confirm they’ve been hit.

#4

Ok nice, one last suggestion;

add a particle emitter when they get hit for better effect and look more flashy

Either way nice job

1 Like
#6

Try out a module called RaycastHitbox if you want to maximize performance / accuracy

#7

Looks alright. Here are a few tips:

  1. The use of defining variables as globals is heavily discouraged. What I mean is this.
-- Not good
a = 5
-- good
local a = 5

This is because of slower referencing times if we are comparing the reference time of variables from the stack and heap. Local variables are stored in the stack while globals are stored in the heap.

  1. Your “Tween” function seems pretty useless and a waste of memory. All it does is run tweenservice:Create() so I see no benefit or extra functionality to this. You are just taking space in the call-stack and heap. Function calls aren’t instantaneous.

  2. It’s considered good practice to define services at the top of your scripts. No need to define TweenService every time the “Tween” function is ran. (Don’t use the “Tween” function tho lol).

  3. :GetTouchingParts() returns an array so you should use ipairs instead of pairs to iterate. ipairs is faster and is meant to be used on arrays. Pairs should be used on only dictionaries.

  4. Your touched connection is empty?

It’s not a bad script just has a few bad practices.
Happy Coding :()

#8

Tween is only for testing and movement purposes.

But thanks I’ll look this all this!

#9

No problem, but I meant that instead of having a custom tween function just use tweenService:Create(). It works the exact same way as your ‘Tween’ function and you even end up inputting the same arguments. But that is not a good idea for stated reasons.