(Not for beginners) Making advanced sword combat

Hm…For the looks of it, This is kinda advanced, Especially the Animations. Good work @JustAGameDeveloper1 !

How does changing elseif to else if benefit at all? just because its more to be in js?

1 Like

I’m pretty sure (and correct me if I’m wrong, it’s 2 am and I’m tired but) the loop is for consecutive swings, if they don’t swing again within 5 seconds, it resets to the first swing - though this is kind of a bad way to do this if this is what it’s actually for.

Would much rather use tick() differences inside of the activated function rather than a loop.

Check:GetPropertyChangedSignal("Value"):Connect(function()
   if Check.Value == 1 or Check == 2 then
       task.wait(5)
        Check.Value = 0
  end
end)

Wouldn’t this work the same way regardless?

Yes but no. Basically, your script fires as soon as it is changed to Value 1 and starts a count, and when it gets to value 2, it starts a separate count.

This means that say they click once, Value goes to 1, they wait 3 seconds, click - then the value hits 2. They only have 2 seconds to execute the next click before the value is reset to 0 since the script is still waiting from the Value == 1.

On top of that, it would start a new timer when you hit value == 2, this could disrupt your value == 1 swing and vice versa.

This is what I would do instead:

local LastSwing
Tool.Activated:Connect(function()
	local humanoid = script.Parent.Parent:WaitForChild("Humanoid")
	if Enable == true then
		Enable = false
		if Check == 0 then
		humanoid:LoadAnimation(RS.Sword.Animations["1"]):Play()
		LastSwing = tick()
		Enable = true
	elseif Check == 1 and (tick()-LastSwing) <= 5  then
		humanoid:LoadAnimation(RS.Sword.Animations["2"]):Play()
		wait(0.5)
		Check = 2
		LastSwing = tick()
		Enable = true
	elseif Check == 2 and (tick()-LastSwing) <= 5 then
		humanoid:LoadAnimation(RS.Sword.Animations["3"]):Play()
		wait(0.5)
		Check = 0
		Enable = true

  		else
			Check = 0 
  		end
	end

end)
2 Likes

It would clean your code immensely and make it easier to read for others (especially since you are creating a tutorial for it).

there are so many unnecessary “ends” and indents that occur using “else if” rather than “elseif” that it just becomes messier.

oh wait i misunderstood, i thought you said elseif was bad, mb

This is pretty good for beginners who are advancing, I personally wouldn’t write code like that, and neither should anyone else!

Here’s some feedback on how to better your coding:

  • Don’t use while loops to set values back to default…

  • Don’t use lots if-else statement.

  • Create variables so you only have to define instance once.

  • Use :WaitForChild() please.

  • VFX / GFX should preferably be clien-side.

  • Don’t ever use wait() over task.wait().

But aside from that, I think you did a decent job. :+1:

4 Likes

New advanced sword combat tutorial!

Includes:
math.random

Better code

Particle slashes

Updating tutorial!

1 Like

Do people really use task.wait() now. I have never used it since it came out. What is the difference?

1 Like

The sword only does damage on the client’s side. The damage won’t pop up for other players, so they won’t actually die.

@epoke466
wait() will be deprecated soon (strongly discouraged against its use), and for good reason. wait() has throttling built into it, which means that at some point doing wait() can actually yield the thread (pause code execution for that block of code) for a lot longer than expected. Additionally, wait() works at 30 hertz (30 frames per second) meaning the minimum time code execution can resume is 1/30 (about 0.033…) seconds.

task.wait() is designed to correct the pitfalls of wait(). It is designed to resume as many task.wait()'s the code has within a single frame without throttling. In other words, any number of task.wait()'s will be resumed when their time is up instead of delaying their resumes to a different frame. Additionally, task.wait() runs at 60 hertz (60 frames per second) meaning the minimum time code execution will resume is 1/60 (twice as fast as wait() ).

@JustAGameDeveloper1
If you can, avoid waiting all together by using event-based programming logic. I can only think of three times (off the top of my head) when task.wait() has legitimate uses:

  1. Creating a continuous while loop if the order doesn’t matter of when it runs every frame
  2. If no event signal exists, but I need to wait a certain amount of time after an action
  3. Some niche cases like waiting a frame for joints to properly align after characteradded is fired

The animation script with the wait()'s doesn’t really have a need to use task.wait() at all if, what I’m assuming, it is only used to wait until an animation has completed. It’s more intuitive to automatically grab the animations regardless of the name or how many animations there are. Loading an animation to the humanoid is deprecated; instead, load the animation directly to the Animator object inside the character’s Humanoid:

--It's better practice to get the service instead
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local player = game:GetService("Players").LocalPlayer
--Accounts for the character existing or not existing
local character = player.Character or player.CharacterAdded:Wait()
--Use WaitForChild because the contents of ReplicatedStorage may not have ...well... replicated yet:
local animationsFolder = ReplicatedStorage:WaitForChild("Sword"):WaitForChild("Animations")
local slashingRemote = ReplicatedStorage.Sword:WaitForChild("Remotes"):WaitForChild("Slashing")
--Definitely not a fan of having scripts inside the Workspace, but I'll just leave it for now
local tool = script.Parent
--Grab the animator inside the character's humanoid
local animator = character:WaitForChild("Humanoid"):WaitForChild("Animator")
--No need to load the animation each time the animation is played
--Let's store the animations inside of an array so we can easily randomize which one is played:
local animationTracks = {}
--Make use of descriptive variable names so it is immensely easier to understand exactly what is happening
local noAnimationPlaying = true
--Simply look through all the animations and have them connect to a local function when the animation stops
--Doing it this way lowers the amount of code we need to update. 

local function AnimationStopped()
	noAnimationPlaying = true
end

--Assuming the "Animations" folder only ever contains animations, it's honestly a better approach to loop through the "Animations" folder
--	and load the animations/set table values so there is absolutely no code that needs to be updated if an animation is ever added/removed/renamed:
for index, animation in pairs(animationsFolder:GetChildren()) do
	--Get children returns an array, so we can use that array's index to store the animationTracks in our own array:
	--Humanoid:LoadAnimation() is deprecated, load the animation to the humanoid's "Animator" object instead:
	local animTrack = animator:LoadAnimation(animation)
	animationTracks[index] = animTrack
	--event-driven way of resetting the debounce regardless of how short/long the animation lasts or which animation was played
	animTrack.Stopped:Connect(AnimationStopped)
end

tool.Activated:Connect(function()
	if noAnimationPlaying then
		noAnimationPlaying = false
		--Randomly choose a different swinging animation each time. This number is automatically seeded
		local chosenAnimation = animationTracks[Random.new():NextInteger(1, #animationTracks)]
		--Don't trust the client. Just send the signal and let the server verify
		slashingRemote:FireServer()
		chosenAnimation:Play()
	end
end)

If the animations MUST be played in order, then try something like this (comments above each added line so it’s easier to find):

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local player = game:GetService("Players").LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local animationsFolder = ReplicatedStorage:WaitForChild("Sword"):WaitForChild("Animations")
local slashingRemote = ReplicatedStorage.Sword:WaitForChild("Remotes"):WaitForChild("Slashing")
local tool = script.Parent
local animator = character:WaitForChild("Humanoid"):WaitForChild("Animator")
local animationTracks = {}
local noAnimationPlaying = true
--To keep track of the current playing animation
local currentAnimationPlaying = 1

local function AnimationStopped()
	--increment the variable by 1
	currentAnimationPlaying += 1
	--if the variable goes higher than the amount of animation tracks, reset the variable back to 1
	if currentAnimationPlaying > #animationTracks then currentAnimationPlaying = 1 end
	noAnimationPlaying = true
end

for index, animation in pairs(animationsFolder:GetChildren()) do
	local animTrack = animator:LoadAnimation(animation)
	--index can't be used for this because :GetChildren() returns instances based on the order they were added, this means they can be in any order.
	-- Since the OP used "1", "2", and "3" as the names, we can just use that by converting it to a number:
	animationTracks[tonumber(animation.Name)] = animTrack
	animTrack.Stopped:Connect(AnimationStopped)
end

tool.Activated:Connect(function()
	if noAnimationPlaying then
		noAnimationPlaying = false
		--Point to the variable we defined that keeps track of the current playing animation
		local chosenAnimation = animationTracks[currentAnimationPlaying]
		slashingRemote:FireServer()
		chosenAnimation:Play()
	end
end)

If the animations have different lengths, yet a consistent cooldown is needed between the attacks, then use one of the time methods in the API to handle the cooldown. If :GetServerTimeNow() is used, then you can even synchronize this cooldown between the client and server (extra work is needed on the server to verify cooldowns):

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local player = game:GetService("Players").LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local animationsFolder = ReplicatedStorage:WaitForChild("Sword"):WaitForChild("Animations")
local slashingRemote = ReplicatedStorage.Sword:WaitForChild("Remotes"):WaitForChild("Slashing")
local tool = script.Parent
local animator = character:WaitForChild("Humanoid"):WaitForChild("Animator")
local animationTracks = {}
local currentAnimationPlaying = 1
--Add a cooldown time to a variable
local cooldown = 0.5
--Create a cooldownEnd time and set it to the current time so the player can immediately begin attacking
local cooldownEnd = game.Workspace:GetServerTimeNow()

for index, animation in pairs(animationsFolder:GetChildren()) do
	local animTrack = animator:LoadAnimation(animation)
	animationTracks[tonumber(animation.Name)] = animTrack
	--No longer need the animation stopped signal because we're using a timed cooldown now
end

tool.Activated:Connect(function()
	--Immediately capture the current time
	local currentTime = game.Workspace:GetServerTimeNow()
	--Check if the cooldown has finished
	if currentTime >= cooldownEnd then
		--reset the cooldownEnd to the current time + the cooldown timer
		cooldownEnd = currentTime + cooldown
		--We no longer need to use a variable to see if an animation is playing or not, the cooldown handles that part
		local chosenAnimation = animationTracks[currentAnimationPlaying]
		--more polish could include accounting for the animations length when doing a cooldown
		print(chosenAnimation.Name)
		slashingRemote:FireServer()
		chosenAnimation:Play()
		--I don't like to have functions with less than 3 lines of code. That's too much abstraction for me
		currentAnimationPlaying += 1
		if currentAnimationPlaying > #animationTracks then currentAnimationPlaying = 1 end
	end
end)

This is only meant as an example to show a more intermediate way of programming that one specific script. @caviarbro, @Vyntrick, @savio14562, @StarJ3M, @Revelted, @Koiyukki, and myself are sharing our knowledge with you in the exact same way you are sharing your knowledge with others. Notice the differences between the first and last code examples I provided; this is deliberate to showcase my actual thought processes and how some of those thoughts were completely scrubbed in the final code. Moral is: nobody instantly knows the best way to solve a problem, we pick at it until we mould it into something we’re satisfied with; be thankful if someone takes time out of their day in an attempt to help mould your script. By the way, how you organize most of your objects is perfect and allows for easy coding!

5 Likes

Thats long.

I will include all of you guys thoughts in the NEW tutorial, all scripts will be mine, no stolen code, i have recently gotten better

1 Like

New sword combat tutorial, coming soon

Includes the following:

Better and shorter code – using if … then.
Free animations – by ME.
Waiting on the task (task.wait()) – i seem is even better then wait() because wait() runs like a potato, while task.wait() is much easier to use.
Customizable Swords – Done with searching for the katana name in ReplicatedStorage.
canDamage boolean values – Instead of using broken damage code, i’m gonna fix the bug where you dont click and it damages anyway. This also uses if … then statements.
Hit Highlights – Isn’t it even BETTER to add a highlight on player hit?
Good FX – This time im gonna share some sword effects with you guys. No more bubble circle!
Hit Animations – Yep. You heard me. This is gonna use values AGAIN.
Loading animations on the Animator – Even better because humanoid:LoadAnimation() is just a yawn

4 Likes

Bump, but somebody liked this. I am sorry for bumping.

The tutorial will release tommorow. I’m truely sorry for bumping. Forgive me. I just wanted to let everybody know.

2 Likes

Some of you guys are gonna say “Stop bumping.” I just wanted to let everybody know that I’m sorry that the edition didn’t release and the code will make more sense. Thank you. Okay? I kinda lied / forgot.

I’m really sorry for bumping! I’ll try to get it done by August 2nd! :sad:

1 Like

you said you’d release it 3 months ago, how did u manage to not release it within those 3 months

I forgot about it due to my games.
babasdbsjhdsjfbjghjkdjgksg

1 Like

before anyone says “YOU BUMPED THIS!!!1!!!1!!!” i am so sorry and i’ll try to oupdate this post not to bump it anymore. Thanks :happy3:

Septemeber is here! I’ve finally come to update my old projects with a new one. Projects that are being updated:

  • Captcha Verifaction: New layout and captchas, probably data-storing so you dont have to do it again.
  • Aqua Mod: Welcome will be more detailed, fixed layout, list of mods and banned users (ID ONLY, NOT FOR MODS)
  • Sword Combat: Slash, sound effects, more detailed animations, and a bunch of other stuff. Stay tuned!
2 Likes

What method will be used for hit detection with the sword?