Hi, I am currently making a sword and it has some noticeable delays between the strikes and damage that is actually dealt. I read an article saying that FindFirstChild takes more time than some other methods, so I am wonder how I can improve this sword script. Thanks for reading my post.
This is the client script
UIS.InputBegan:Connect(function(input, gameProcessed)
if input.UserInputType == Enum.UserInputType.MouseButton1 and not gameProcessed and debounce == true then
print("left clicked!")
print(rankValue)
wait(0.1)
if char.RightHand:FindFirstChild("Weld") and debounce == true then
debounce = false
print("sword found ready for slash")
loadedAnim:Play()
dmgpart.Touched:Connect(function(hit)
if db2 == true then
db2 = false
if hit.Parent:FindFirstChild("Humanoid")then
activateSwordDmg:FireServer(hit, sideValue.Value)
else
print('hit the wrong thing')
end
end
end)
wait(0.5)
db2 = true
debounce = true
wait()
else
print("okay m8")
end
end
end)
This might be a good old case of latency. The time it takes for the server to get the request and then do the damage, then send that information back to the client is always a bit noticeable. Although, you also have a wait(0.1) at the start that probably contributes to the feel of it.
First though, I’ve made some changes to the code to explain a different way to approach this (untested):
local ready, swinging = false, false
-- the event is only created once, as otherwise you end up with damage stacking
-- if your game features equipping/unequipping weapons, you'll want to
-- disconnect the old event and connect a new one when that happens
dmgpart.Touched:Connect(function(hit)
if swinging and hit.Parent:FindFirstChild("Humanoid") then
-- this is vulnerable to exploiting, see more in the explanation below
activateSwordDmg:FireServer(hit, sideValue.Value)
end
end)
UIS.InputBegan:Connect(function(input, gameProcessed)
-- long conditions which just exit out can be made into a guard clause
if ready or gameProcessed or input.UserInputType ~= Enum.UserInputType.MouseButton1 then return end
print("left clicked")
print(rankValue)
-- wait(0.1) -- This wait is not necessary and should be removed.
if not char.RightHand:FindFirstChild('Weld') then
print('no sword equipped')
return
end
-- these flags tell the event when it can hit
ready = true
swinging = true
-- instead of waiting about 0.5 seconds (which is inaccurate anyways), you can use
-- the `.Stopped` event from the AnimationTrack to `:Wait()` for it to finish
loadedAnim:Play()
loadedAnim.Stopped:Wait()
swinging = false
-- you can add some yield here to have a cooldown between swings
ready = false
end)
Alongside the comments explaining some of the changes, I’d like to go into how events work in Roblox. There is a trailing wait() in your client script; it happens at the end of the UIS event. Events run asynchronously, so waiting inside one of them won’t actually stop code anywhere else. Trailing waits just make the thread stick around for a little longer but offer no observable functionality.
On top of that, you shouldn’t use wait() to solve problems that it seemingly does. This thread goes more in depth with that:
In most cases, there is an equivalent Roblox event that you can listen to via Connection:Wait() (different from wait()) which more efficiently does the trick. I assumed in your example that you wanted to wait until after the animation was over, but this might not be the case. Maybe you want it to wait until a certain keyframe is reached, for example. There is also an event for that, and it’s always worth checking the DevHub if it can make the code a little nicer.
Next up is the event and debouncing. I changed the code as explained in the comment. The problem is that once you :Connect() an event, it’s not disconnected until you explicitly call :Disconnect(), or the object it was connected to is :Destroy()ed. As such, every time the player swings a new connection is made, and more damage is tacked on to the hit.
The swinging debounce controls when the sword can do damage, and the ready controls when the sword is ready to be swung again. This avoids multiple connections.
Lastly, I want to address the RemoteEvent. You are giving a bit too much authority to the player. The server code should also keep track of its own debounce regarding the player’s last hit. It shouldn’t be set by the client, as this is exploitable. You’ll also want to make sure that hit (or hit.Parent) is actually an instance (via typeof()), as people can pass tables that look like instances. And, of course, you’ll want some sort of distance check. Simply testing if the player’s character is within some small radius from their target (with some leniency to compensate for lag) will mitigate the issue.
Hi, I just messed around with more testing and the code you provided seems to hit the humanoid more than once. I am not super advanced in scripting and I would need more help. Could you please help me fix this behavior? Thanks!