How I could improve my combat system code

Help and Feedback Scripting Support
#1

Hello! I would like to know how I could improve my combat system code
I have a combat system code implemented and I would like to receive tips and help to improve it, could you give me your feedback, please! Thank you very much :smiley:

Localscript

-- Declared
local player = game.Players.LocalPlayer
repeat wait() until player.Character.Humanoid
local humanoid = player.Character.Humanoid
local stopAnim = player.Character
local mouse = player:GetMouse()

-- Defense animation
local defanim = Instance.new("Animation")
defanim.AnimationId = "rbxassetid://6229874629"

-- Light hit animation
local light_hit_anim = Instance.new("Animation")
light_hit_anim.AnimationId = "rbxassetid://6232686373"

-- Concentrate hit animation
local con_hit_anim = Instance.new("Animation")
con_hit_anim.AnimationId = "rbxassetid://6232702541"

-- Input system
UserInputService = game:GetService("UserInputService")
local player = game.Players.LocalPlayer
local mouse = player:GetMouse()

-- Defense anim F key
UserInputService.InputBegan:Connect(function(input, gameProccesedEvent)
	if input.KeyCode == Enum.KeyCode.F then
		print("Playing defense animation! done.")
		local playAnim = humanoid:LoadAnimation(defanim)
		playAnim:Play()
		script.DefAnim:FireServer()
	end
end)

-- Light attack anim with the left mouse click:
UserInputService.InputBegan:Connect(function(input, gameProccesedEvent)
	if input.UserInputType == Enum.UserInputType.MouseButton1 then
		print("LMB pressed, playing the anim")
		local playAnim = humanoid:LoadAnimation(light_hit_anim)
		playAnim:Play()
		script.LightHitAnim:FireServer()
	end
end)

-- Concentrate attack anim with the right mouse click:
UserInputService.InputBegan:Connect(function(input, gameProccesedEvent)
	if input.UserInputType == Enum.UserInputType.MouseButton2 then
		print("LMB pressed, playing the anim")
		local playAnim = humanoid:LoadAnimation(con_hit_anim)
		playAnim:Play()
		script.BigHitAnim:FireServer()
	end
end)  		-- THAT ARE EVENTS

ServerScript

-- SERVER CONECCTION

-- Defense anim connected to server
script.Parent.Animations.DefAnim.OnServerEvent:Connect(function()
	print("Connected to defense anim.")
end)

-- Light hit anim connected to server
script.Parent.Animations.LightHitAnim.OnServerEvent:Connect(function(plr)
	print("Connected to light hit anim.")
	
	-- Damage functions
	local AttackDist = 10 --If a player is further than this value, they won't be damaged.

	for i,v in pairs(game.Players:GetChildren()) do --Loops through all players.		
		if v.Character ~= nil and plr.Character ~= nil and v ~= plr then --Checks if there's a character
			local pRoot = plr.Character:FindFirstChild("HumanoidRootPart")
			local vRoot = v.Character:FindFirstChild("HumanoidRootPart")

			if pRoot and vRoot and (plr.Character.HumanoidRootPart.Position - v.Character.HumanoidRootPart.Position).Magnitude < AttackDist then --Checks if the Root parts exist and if the player is in range.
				local Hum = v.Character:FindFirstChild("Humanoid")

				if Hum and Hum.Health > 0 then --Checks if player is alive.
					Hum.Health = Hum.Health - 10 --Change "100" to the damage the player should take.
				end

				--Stuff that happens when a player is in range.
			end
		end
	end
	
end)

-- Concentrate big hit anim connected to server
script.Parent.Animations.BigHitAnim.OnServerEvent:Connect(function(plr)
	print("Connected to concentrate hit anim.")
	
	-- Damage functions
	local AttackDist = 10 --If a player is further than this value, they won't be damaged.

	for i,v in pairs(game.Players:GetChildren()) do --Loops through all players.		
		if v.Character ~= nil and plr.Character ~= nil and v ~= plr then --Checks if there's a character
			local pRoot = plr.Character:FindFirstChild("HumanoidRootPart")
			local vRoot = v.Character:FindFirstChild("HumanoidRootPart")

			if pRoot and vRoot and (plr.Character.HumanoidRootPart.Position - v.Character.HumanoidRootPart.Position).Magnitude < AttackDist then --Checks if the Root parts exist and if the player is in range.
				local Hum = v.Character:FindFirstChild("Humanoid")

				if Hum and Hum.Health > 0 then --Checks if player is alive.
					Hum.Health = Hum.Health - 25 --Change "100" to the damage the player should take.
				end

				--Stuff that happens when a player is in range.
			end
		end
	end
end)
1 Like
#2

Immediate thing I notice is you’re using the depreciated animation setup. Many many many people use this, and you could still keep using it, but as it’s depreciated it’d be best to move on. Deprecating LoadAnimation on Humanoid and AnimationController

Don’t set the humanoids health. As a general rule you can observe FF using humanoid:TakeDamage() instead. While maybe you’re not using FF, it’s smart to use that anyway.

1 Like
#3

This would be better suited for #help-and-feedback:code-review

You should use player.CharacterAdded:Wait() instead of a loop and save it to a variable

local character = player.Character or player.CharacterAdded:Wait()
-- if the player's character already loaded the get it, otherwise wait for it
-- when it does load, it'll save it to the variable

I’m a little confused on why you re-declared player and mouse. You could just reuse the variables that were created before. Also, you should make UserInputService local

local UserInputService = game:GetService("UserInputService") 
-- making variables local is faster because of the way they're utilized

@Conmmander already pointed out the animation loading and the Humanoid:TakeDamage(float damage)

For the UserInputService event connection, add

if gameProccesedEvent then return end 
-- this is to prevent the script from detecting input if the player is typing in a TextBox or chat

You should use game.Players:GetPlayers() instead of GetChildren() because GetPlayers() it returns every object with the Player class instead of any object.

Another thing I would like to add is that the animations should be done server-side

#4

How could i implement it? something like this?

local player = player.CharacterAdded:Wait()
local humanoid = player.Character.Humanoid
local stopAnim = player.Character

is it good?

#5

How could i implement it in my code? could you help me with that please :frowning:

#6

No, that returns the character

local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local humanoid = character.Humanoid
local stopAnim = player.Character -- also, why is this directed to the player's character?
#7

I would disagree with implementing animations server side.

The advantage to animations via the server aren’t much different than that of doing it on the client. Sure someone could play fake animations and such, but really I think as the animations are on that specific player, they’d be better suited on the client. Gun scripters also follow that same logic, hence why any gun animation is typically on the client.

1 Like