Combat one shotting?

Help and Feedback Scripting Support
1

Im trying to make a combat script, and it works fine in studio… but in game it one shots everyone… and i have no clue why
i tried to fix it, but after an hour of stressing over it i decided to come here and see if anyone could possibly point out any flaws in my code

local module = {}

local anims = {"rbxassetid://4503665409";
		"rbxassetid://04806050820";
		}
local animation = script.Animation


function module.Punch(player)
local yaboi = {}
local myboi = {}
local char = player.Character
local leftArm = char:FindFirstChild("Left Arm")
local rightArm = char:FindFirstChild("Right Arm")
local data = player.Data
animation.AnimationId = anims[math.random(1,#anims)]
local animPlayer = char:FindFirstChild("Humanoid"):LoadAnimation(animation)
animPlayer:Play()
data.XP.Value = data.XP.Value + math.random(1,20)
if animation.AnimationId == anims[1] then
leftArm.Touched:Connect(function(hit)
	if yaboi[player] ~= true then
		yaboi[player] = false
		if hit.Parent:FindFirstChild("Humanoid") then
		local hum = hit.Parent:FindFirstChild("Humanoid")
			if hum.Parent.Name ~= player.Name then
				wait(1)	
				hum:TakeDamage(data.Strength.Value * 2 + math.random(1,20))	
				
				local tago = Instance.new('ObjectValue',hit.Parent.Humanoid)
				tago.Name = 'creator'
				tago.Value = player
					game.Debris:AddItem(tago,.25)		
					
			wait(1)
			yaboi[player] = true				
			end	
		
		end
	end
end)
	else
	if animation.AnimationId == anims[2] then
	rightArm.Touched:Connect(function(hit)
	if myboi[player] ~= true then
		myboi[player] = false
		if hit.Parent:FindFirstChild("Humanoid") then
		local hum = hit.Parent:FindFirstChild("Humanoid")
			if hum.Parent.Name ~= player.Name then
			
				hum:TakeDamage(data.Strength.Value * 2 + math.random(1,20))	
				
				local tag = Instance.new('ObjectValue',hit.Parent.Humanoid)
				tag.Name = 'creator'
				tag.Value = player
					game.Debris:AddItem(tag,.25)
					wait(1)
				myboi[player] = true	
					
			end

			
		end
		
	end
end)
		end
	end
end

return module
1 Like
3

Did you know that connections don’t magically disappear? After punching for long enough, you will detect hits multiple times.
Disconnect the connections or use a different form of hit detection. Putting a print statement in the event should show this.
You also have memory leaks because of this. (yaboi and myboi)

5 Likes
8

Rather than being immature like both of you, and arguing, I’m going to offer a solution.

As @Wunder_Wulfe stated, connections can stack, they will continue to run even if you create a new one, the only way to cancel them out, is by calling the Disconnect function on them. This will cancel the event completely. This is a common flaw in a lot of scripts, and it results in numerous memory leaks.

You can read up more on this here, Documentation - Roblox Creator Hub

Each time the user punches you’re creating a new connection for the Touched event.

You could either only connect “Touched” once, or you can disconnect the old version each time you punch.

Disconnecting the event each punch

To do this we’ll need to store the touched event in a table, then before creating another Touched Connection we’ll remove the old one.

local touchedConnections = {} -- Creating a table where we'll store all of our punches

Now before creating a new connection we need to check if the user already has a punch, if they do then we’ll disconnect it, after disconnecting it we’ll save the new connection to the table.

if touchedConnections[player] then -- Checking too see if the connection is already existing
   touchedConnections[player]:Disconnect() -- Disconnecting it
   touchedConnections[player] = nil -- Defining the index of the player too nil
end

local newConnection = leftArm.Touched:Connect(function(hit) -- Assigning "newConnection" to the Connect event
   -- Handle the touch here
end)

touchedConnections[player] = newConnection -- Saving the new connection

Checking if an event already exists

Doing it this way is pretty much the exact same as doing it the other way, minus some steps. We’ll still need to create a table to hold all of our connections.

local touchedConnections = {} -- Creating a table where we'll store all of our punches

Once we do this we simply check if a user already has a connection, if they don’t then make a new one and store it in the table.

Please note, that if the user dies, leaves, etc… you will most likely need to make changes in order to deal with that! Ie; disconnect the event when the user dies and create a new one (as the arm changes)

if not touchedConnections[player] then -- Checking if the user has a touched event connection stored
   local newConnection = leftArm.Touched:Connect(function(hit) -- Defining newConnection once again
      -- Handle the touching here, once again
   end)
   touchedConnections[player] = newConnection -- Saving our connection.
end

If you have any questions please be sure to ask below!

4 Likes
9

Adding on, you could also just disconnect them after a timer or within the connection itself after the apparent debounce which would not require all these arrays and etc.
Hopefully they do also notice the memory leak that could also slog their games performance down to a halt.

10

I mean… I guess you “could” but this will result in a lot of unnecessary threads. You’ll need to take in to account the server & the client aren’t exactly in sync as well.

The idea of creating a timer would end up being a lot more work to prevent things such as “Double Hitting” where the user will do twice the damage due to the connection still being there.

1 Like
11

No i am talking about the fact that he is using a timer in his connection where the debounce is and he could just disconnect it after it finishes. I don’t see what you mean by syncing issues because if syncing was an important problem here then they wouldn’t be using Touched, at least not on the server.

12

I believe what he means by this is the factor of the slight delay in between the client sending information to the server.

Creating a timer would most likely raise the chances of unwanted occurrences, such as double-hitting or hits not registering.

13

Yes but in this case he has a debounce and a timer within the debounce which is what I was talking about using.
From the way they wrote their code, if they hit somebody then they deal damage and wait before allowing it to happen again. (Ignoring the connection stacking). Yes they could always have touched connections bound to detect hits all the time and when punching but it might be unnecessary. If for some reason they wish to keep using Touched, my suggestion was just to disconnect after clocking in the hit (or if they didnt hit anything, after the animation is over) and then handling the cooldown

15

ok so i tried what you said, it seems to work somewhat after about 20 minutes of trying to understand it, but some hits arent registering, and the touch function still goes through multiple times, but it doesnt go through as fast, also it only one shots actual players, which i find weird? other than that it works fine on NPCs, but thats not what im aiming for, im trying to get an efficient pvp combat going. if you could point out where the flaw is, it would be greatly appreciated. here is the code

local module = {}

local anims = {"rbxassetid://4503665409";
		"rbxassetid://04806050820";
		}
local animation = script.Animation


function module.Punch(player)
	
local yaboi = {}
local myboi = {}

local char = player.Character
local leftArm = char:FindFirstChild("Left Arm")
local rightArm = char:FindFirstChild("Right Arm")

local data = player.Data

animation.AnimationId = anims[math.random(1,#anims)]
local animPlayer = char:FindFirstChild("Humanoid"):LoadAnimation(animation)
animPlayer:Play()

data.XP.Value = data.XP.Value + math.random(1,20)


local touchedConnections = {}

if animation.AnimationId == anims[1] then
	
if touchedConnections[player] then -- Checking too see if the connection is already existing
   touchedConnections[player]:Disconnect() -- Disconnecting it
   touchedConnections[player] = nil -- Defining the index of the player too nil
end
if not touchedConnections[player] then
local newConnection = leftArm.Touched:Connect(function(hit)
	if yaboi[player] ~= true then
		yaboi[player] = false
		if hit.Parent:FindFirstChild("Humanoid") then
		local hum = hit.Parent:FindFirstChild("Humanoid")
			if hum.Parent.Name ~= player.Name then
				wait(1)	
				hum:TakeDamage(data.Strength.Value * 2 + math.random(1,20))	
				
				local tago = Instance.new('ObjectValue',hit.Parent.Humanoid)
				tago.Name = 'creator'
				tago.Value = player
					game.Debris:AddItem(tago,.25)		
					
			wait(1)
			
			yaboi[player] = true				
			end	
		
		end
	end
end)
touchedConnections[player] = newConnection
	else
	if animation.AnimationId == anims[2] then
	if touchedConnections[player] then -- Checking too see if the connection is already existing
   touchedConnections[player]:Disconnect() -- Disconnecting it
   touchedConnections[player] = nil -- Defining the index of the player too nil
end
if not touchedConnections[player] then
	local newConnection = rightArm.Touched:Connect(function(hit)
	if myboi[player] ~= true then
		myboi[player] = false
		if hit.Parent:FindFirstChild("Humanoid") then
		local hum = hit.Parent:FindFirstChild("Humanoid")
			if hum.Parent.Name ~= player.Name then
			
				hum:TakeDamage(data.Strength.Value * 2 + math.random(1,20))	
				
				local tag = Instance.new('ObjectValue',hit.Parent.Humanoid)
				tag.Name = 'creator'
				tag.Value = player
					game.Debris:AddItem(tag,.25)
					wait(1)
					
				myboi[player] = true	
						
			end
		end
	end
end)
touchedConnections[player] = newConnection
				end
			end
		end
	end
end

return module
16

i think my server script could also be flawed, im not sure so im just going to put it here and if you see any flaws, just hit me up and let me know

local event = game.ReplicatedStorage.Server
local module = require(game.ReplicatedStorage.Combat.Cmbt)
local coolio = {}
event.OnServerEvent:Connect(function(player,check)
	if check == "combat" then
		if coolio[player] ~= true then
			coolio[player] = true
			module.Punch(player)
			wait(.5)
			coolio[player] = false
		end
	end
end)