What's the most efficient way to code a combat system

Help and Feedback Code Review
#1

Hey guys, I’m Ehcode. I’m an aspiring solo developer and my goal is to become a jack of all trades on this amazing platform.Between late 2017 till now, I’ve been learning how to code but I’ve realized that my code usually lacks efficiency and that there is always a better way to things.I finally decided to work on my own game but I’m having a hard time deciding on what’s the most efficient way to code my combat system. here’s a short summary of how my system works:

--local script

local mouse = Player:GetMouse()
local CombatRemote = game.ReplicatedStorage.CombatRemote

mouse.MouseButton1Down:Connect(function()
CombatRemote:FireServer()
end)

 --Server script


 local combatRemote = game.ReplicatedStorage.CombatRemote
 

 combatRemote.OnServerEvent:Connect(function(Player)
 local Char = Player.Character
 local Humanoid = Char.Humanoid
 local DamageScript = script.DamageScript
 
 local Anim1 = Humanoid:LoadAnimation(script.Anim1)
 local Anim2 = Humanoid:LoadAnimation(script.Anim2)
 local Anim3 = Humanoid:LoadAnimation(script.Anim3)
 
 local Anims = {Anim1, Anime2, Anim3}
 --here I would then randomize the animation and load Animations[1] into the character
 
Anims[1]:Play()
 if Anims[1] == Anims1 then
 local clone = DamageScript:Clone()
 clone.Parent = BodyPart1
 game.Debris:AddItem(clone)

 elseif Anims[1] == Anims2 then
 --same as above but different body part
 elseif Anims[1] == Anims3 then
 --same as above but different body part
 end
 end)
``
Tips on how I can make a better/more efficient system are appreciated!
Sorry for the rushed post :(
9 Likes
#2

Few things I’d change is:

  • You are loading the animations and then getting random one. The better way would be to get random id and then load it in.

  • You seem to use a lot of if statements, you should probably organize your code in a better way so it’s easily expandable, for your case, something like this:

local asd = {
  Animation1 = {
    id = 123456789,
    bodyPart = BodyPart1
  }
};

local anim = Instance.new('Animation')
anim.AnimationId = "rbxassetid://%d":format(asd.Animation1.id)
humanoid:LoadAnimation(anim):Play()

clone.Parent = asd.Animation1.bodyPart
  • One more thing I’d change is the fact that you parent the script to bodypart for what it seems is just dealing damage. You should probably keep it all in one place for something like that.
5 Likes
#3

Thanks I’ll try this method! :smiley: I’m also sorry you had to read through my sloppy post.I accidentally published it before I was done.

#4

Proper codeblocks are created with three backticks. Please use that from now-on when you are posting code to the forums.

```lua
-- Your code
```

will become

-- Your code

The title and contents of your post are slightly contradictory. What is it explicitly that you are trying to achieve: the creation of a combat system or the improvement of your code above?

In terms of the code you posted above though, I’ve got a few things of my own:

  • Your animation variables should be converted into a table. Instead of loading ones you will and won’t need, load only the one intended to play during this sequence.
  • You are too reliant on the server. Push some work off to the client, such as the playing of animations.
  • Don’t use an external script to damage, use a method of the Humanoid to decrement health (Humanoid:TakeDamage or Humanoid.Health = Humanoid.Health - Decrement)
  • Consolidate your code and make it reusable. Nested if statements is bad practice and will impede the readability, organisation and scalability of your code.
5 Likes
#5

Okay that’s going to be very useful in the future, thanks!

The purpose of this post was to find out what other users use for their combat system.Sorry for the confusion haha.

I get most of your points but I have one question:

How would I let the server know which animation played?For example lets say a right punch was the animation that loaded, I’ll need to trigger a touchedevent to look out for when it’s touched so it’d be able to do damage won’t I?How do I do that on the server?

#6

Don’t rely on determining when the animation was played, make the client-server communication as simultaneous as you can. Have the client run their animation, while requesting that the server do something. Alternatively, you can handle such Touched events on the client and allow the server to verify the legitimacy of the request.

#7

In what way could I get the server to “verify the legitimacy”?

#8

The server can check the proximity of the part towards the player (high proximity = client is trying to act on a part that is far away from it, probably indicating an exploiter or malformed request) or whether or not the part is intersecting or touching another.

2 Likes
#9

Oh I get it now, this was very helpful dude.Thanks for putting up with me! :heart:

#11

I too started learning Roblox Lua by making a simple combat system. An important part of any combat system is snappiness and responsiveness. I suggest you load animations on the client. When they press the mouse, play the animation (via their local script). In general, I reserve the server scripts for verifying attacks/damages and creating/modifying objects that need to be replicated.

For hit detection, you could attach a listener (.Touched) on the client and verify it with the server, or do it all on the server. In addition to hit detection of .Touched, you can also use ray tracing for some ranged attacks. The roblox wiki elaborates on all this. Hope this points you in the right direction!

3 Likes
#12

No idea what ray tracing is but I’ll look in to it.This reply was very informative thanks man.:smiley:

#13

Sometimes .Touched can seem fickle for the player, a magnitude check of 0.5 stud on any part of the opponent can make a combat system seem to have much more flow and responsiveness.

Also, the server should generally only handle damage unless you need an effect to replicate to other clients. So an animation, as many have already told you, should be loaded and played on the client. Which may seem counter to what i said before but when an animation plays on the client, it will usually replicate to everything else. Dunno why roblox did it like this, it could be exploited but for the time being thats how it is.

3 Likes
#14

That is because they are generally loaded into the humanoid and players have network ownership of their character

Makes it smoother, in fact all animations and player movement is handled locally

1 Like
#15

Very informative, thanks!

#16

OP is going to be reusing the animations, is it really efficient to be loading the animation every time he wants to attack, when he could just load it in once?

please correct me if I’m wrong and missing something

1 Like
#17

I’d recommend using module scripts. I’m currently in the middle of making a combat game that realises on the use of modules to make chunks of code easier to manage and fix. Stuff like loading/playing animations can be done in a single module, and then just have a LocalScript inside the weapon that runs the module and the set animation when required.

Perks of this is if you had hundreds of weapons, and the animation script doesn’t work, or whatever, you’d have to go through every single script to fix it. Keeping each weapon to just having a Local script to deal with firing the animations module, and then a server script that handles the touching (and linking that to a module as well) is what I’d highly recommend. I can give you more information and help if you need it :+1:

5 Likes
#18

Great reply!Sadly I have little to no experience with module scripts.I don’t think I’d be comfortable using modules but I guess I’ll give it a shot.

#19

I believe the roblox wiki has a tutorial on module scripts. But unless you will be using that module script for multiple other scripts (and are planning to expand the combat system) it’s best to just consolidate all your programming to one core server/local script. Module scripts are nice, but not essential.

1 Like
#21

Here’s just an example, but this is what I have inside my Animations module script (which is located inside RepStorage)

local animations = {}

function animations:Load(animationID, humanoid)
	local animation = Instance.new('Animation')
	animation.AnimationId = animationID
	if not animation then return end
	
	if not humanoid then return end
	
	local loadedAnimation = humanoid:LoadAnimation(animation)
	if not loadedAnimation then return end
	
	loadedAnimation:Play()
end

return animations

Then in a server script inside the tool, you fire a RemoteEvent (which is also inside the tool) so that way the local script can ‘play’ the animation.

tool.Activated:Connect(function()	
	event:FireClient(player, animationID)
end)

Then in the local script inside the weapon:

local replicatedStorage = game:GetService('ReplicatedStorage')
local animations = require(replicatedStorage.Animations)

local tool = script.Parent
local event = tool:WaitForChild('Event')

event.OnClientEvent:Connect(function(animationID)
	animations:Load(animationID, humanoid)
end)

This way, all you’d have to change is the animations ID in the server script, and it would work with all weapons. This means that the same code isn’t repeated all the time

4 Likes
#22

This is my answer, I think it’s probably the most efficient way of doing it.

local Animations = {
  Animation1 = {
    ["ID"] = 123456789,
    ["BodyPart"] = BodyPart1
  },
  Animation2 = {
     ["ID"] = 123456789,
     ["BodyPart"] = BodyPart2
   },
   Animation3 = {
     ["ID"] = 123456789,
     ["BodyPart"] = BodyPart3
   }
};

-- Put this in the event
local AnimationIndex = {}

for Index in Animations do
       if Index then
              table.insert(AnimationIndex, Index)
       end
end

local RandomIndex = AnimationIndex[math.random(1, #AnimationIndex)]

local anim = Instance.new('Animation')
Animatiom.AnimationId = "rbxassetid://" .. Animations[AnimationIndex]["ID"]
local LoadedAnimation = Humanoid:LoadAnimation(Animation)
LoadedAnimation:Play()

local Clone = DamageScript:Clone()
Clone.Parent = BodyPart
1 Like