How would i make this combat system more optimized

Help and Feedback Scripting Support
#1

Hello, i’m currently working on a combat system and i’m having some trouble with how i should proceed.

I’m using “.Touched” for my hitbox. Now the main problem is that when two players or more are fighting each other, they will end up canceling each other out if they punch at the same time.

What i’m trying to do is that if the player gets hit and the other player in in the middle of a punch, the other players hitbox will get destroyed. Problem is that it is not working.

Here is the part of the script where it checks–

-- Code --
Hitbox.Touched:Connect(function(Hit)
		if Hitbox:IsA("BasePart") then
			if not Hit:IsDescendantOf(Character) then
				local enemy = Hit.Parent
				local enemyHumanoid = enemy:FindFirstChild("Humanoid")
				local enemyHumrp = enemy:FindFirstChild("HumanoidRootPart")
					
				if enemyHumanoid and enemyHumrp then
					
					spawn(function()
						local enemyHitboxF = enemy:WaitForChild("CombatFolder",1)
						pcall(function()
							local enemyHitbox = enemyHitboxF:WaitForChild("Hitbox",1)
							if enemyHitbox then
								enemyHitbox:Destroy()
							end
					
						end)
					end)

Now i basically know the problem. The problem is that since i’m using spawn(function), it will continue on with the rest of the script. But if i don’t use it the combat system will become glitchy as hell and damage the player multiple times in one punch.

Any suggestions on how i can improve it? Do i need to use something else than .Changed?

Thank you in advance :slight_smile:

2 Likes
#2

Note: Perhaps you should move this topic to #help-and-feedback:code-review ?

Perhaps you should consider running some coroutines to improve the flow and integrity of multiple threads running simulatenously instead of using spawn(). This will also allow you to have greater control over running code, as you are able to yield them at your will (which would be helpful if you want to terminate some code that would hurt another player, if the player in question is already being hurt for example).

See this article for further details: Documentation - Roblox Creator Hub

Hope this helps,
-Tom :slight_smile:

2 Likes
#3

Could you clarify the issue a little more in-depth?

I understand that the current behavior isn’t what you want and I understand what you’re trying to achieve, but I’m not sure what specific effect the code is producing that you dislike given you only saying:

There’s really no issue with using spawn as a method of asynchronous code other than lack of control, so I’m just not understanding what your specific issue is (i.e. what is it doing that it should or should not be doing)

2 Likes
#4

Okay, so basically.

As stated in my post, I’m trying to make it so when (example)

player1 Punches player2 while player2 is also in a middle of punch
player2’s hitbox will get destroyed to avoid that both can hit each other at the same time.

That is the general idea.

The problem with the code is that it does not delete the hitbox for player2 because it keeps running the rest of the code. What i’m pretty sure about what is happening is that it does not find the hitbox before the enemy has landed a successful hit.

Hope that clears up what the problem is a little :slight_smile:

1 Like
#5

Here’s some info about why you shouldn’t use spawn:

Please do not use spawn . It’s a hold-over from Roblox’s 30hz pipeline and has no guarantees about when (or if) it will begin executing the new thread. spawn is especially evil because it seems innocent at first, but once you have a lot of code that all uses spawn , you can see significant delays in execution upwards of several seconds in a resource-intensive game. Roblox throttles execution of threads and if the device is too bogged down, your thread could just get deferred forever.

This is bad advice. It’s easy to answer when you should use spawn: never. There is never a situation where you want to run some code “maybe at some point in the future or never”. Even for things that seem inconsequential, you can end up shooting yourself in the foot if things run in an order that you don’t expect them to.

Don’t set yourself up for failure. Use coroutine.wrap if you need to begin a new thread. Never use spawn , delay , or wait() in your code.

There’s a full post here: Coroutines V.S. Spawn()... Which one should I use? - #8 by evaera

1 Like
#6

Thank you for the advice,

I will read up on coroutines

2 Likes
#7

I don’t think I’ve ever run into any issues with spawn, but thanks for the thread and quotes, I’ll definitely avoid using it. It’s become more of a nasty habit than anything else, lol.

3 Likes
#8

Is your damage handling taking place after you spawn the new thread?

Also,

You can more than likely set up some kind of debounce or sanity check to prevent this issue from happening. Either setting and checking if the punch has already landed, or performing some kind of tick calculation between when the hit landed and the last hit (or some other method of your preference).

2 Likes
#9

Yes my damage handling is taking place after the new thread. Should i rather do it before?

I’ll try to make a check like you suggested.

1 Like
#10

Probably not,

What effect is the current script having on the actual player?

Say player1 is in the middle of throwing a punch, then player2 initiates a punch. What is the desired outcome of this, and what is the current outcome of your script?

1 Like
#11

Okay so i have created a boolvalue further down in the script that parents to the player that the hitbox hits. This boolvalue is == stunned.
On the client side it checks if the player has that value, and if it does, it will prevent the player from activating the server side of the script (what i’m currently having issues with).

The desired outcome is that when the enemy player gets hit, it will cancel out the ongoing punch from the enemy player if they are in a middle of one. The current outcome is if both presses simultaneously or time difference is very slim, both players will land the punch at each other and stun each other.

Now that is not really a desirable outcome because it will cancel out the combo for player1 if he lands first

1 Like
#12

You should make this check on the server, not the client. The client should not be the one deciding whether or not something should be done, as exploiters could have a field day.

Given what you’ve told me, the new thread might not even be necessary. You should let your checks play out in order rather than asynchronously. (e.g. here’s a sample without the WaitForChild, which I assume is the reason that you’re using spawn):

if enemyHumanoid and enemyHumrp then
	local enemyHitboxF = enemy:FindFirstChild("CombatFolder")

	if enemyHitboxF then
		local enemyHitbox = enemyHitboxF:FindFirstChild("Hitbox")
		
		if enemyHitbox then
			enemyHitbox:Destroy()
		end
	end

	-- rest of code

This is assuming you don’t need WaitForChild. I don’t know how the rest of your systems work, so I’m unsure what effect this will have, but the principle is that you shouldn’t run your checks like that in another thread and expect them to run synchronously, as there’s no guarantee that will be the case.

1 Like