Sword Code Review?

Help and Feedback Code Review
1

Hey, I’ve created a Sword with one skill with a SkillGUI, the only issue is that I feel like both client/server scripts are messy and I feel a way to improve it however I don’t know how
One of the ways I tried to make the code less messy is create functions instead of repeating myself.
How could I shorten or make this code less messy? Thanks.

Client Script:

local UIS = game:GetService("UserInputService")

local TweenService = game:GetService("TweenService")

local Tool = script.Parent
local Events = Tool:WaitForChild("Events")
local AttackEvent = Events.Attack1Event

local Animations = script.Parent.Animations:GetChildren()

local Equipped = false

local player = game.Players.LocalPlayer
local PlayerGui = player.PlayerGui
local CombatGUI = PlayerGui:WaitForChild("SwordCombat")
local AttackFrame = CombatGUI.AttackFrame

local SkillOne = AttackFrame.SkillOne
local BarOne = SkillOne.Bar

local Sounds = Tool.Sounds
local EquipSound = Sounds.Equip
local UnequipSound = Sounds.Unequip

local AttackCooldown = false

local Configurations = Tool.Configurations
local SkillCooldown = Configurations.Skill1Cooldown.Value

local Trail = Tool.Handle.Trail

local Colors = { -- Colors for the GUI
	Attacking = Color3.fromRGB(173, 173, 173);
	Idle = Color3.fromRGB(85, 85, 85);
}

local function ToolStatus(status) -- Handles the attack GUI
	if status == "Equip" then
		Equipped = true
		AttackFrame.Visible = true
		EquipSound:Play()
	elseif status == "Unequip" then
		Equipped = false
		AttackFrame.Visible = false
		UnequipSound:Play()
	end
end

local function ChangeColors(SkillSlot)
	SkillSlot.BackgroundColor3 = Colors.Attacking
	wait(0.05)
	SkillSlot.BackgroundColor3 = Colors.Idle -- Changes the AttackingGUI Color 
end

local function Skill(Bar,Event,EventParams)
	Trail.Enabled = true
	ChangeColors(Bar)
	Bar.Size = UDim2.new(0,0,1,0)
	local BarTween = TweenService:Create(Bar,TweenInfo.new(1,Enum.EasingStyle.Linear,Enum.EasingDirection.In),{Size = UDim2.new(1, 0,1, 0)})
	BarTween:Play()
	Event:FireServer(EventParams)  -- Fire to server for attacking
	Trail.Enabled = false
end

Tool.Equipped:Connect(function()
	ToolStatus("Equip")
end)

Tool.Unequipped:Connect(function()
	ToolStatus("Unequip")
end)

UIS.InputBegan:Connect(function(key)
	if Equipped == true then
		if key.UserInputType == Enum.UserInputType.MouseButton1 and AttackCooldown == false then
			AttackCooldown = true
			Skill(BarOne,AttackEvent,Animations[math.random(1,#Animations)])
			wait(SkillCooldown)
			AttackCooldown = false
		end
	end
end)

Server Script:

local RayCast = require(game.ServerScriptService.RaycastHitboxV3)

local Tool = script.Parent

local Events = Tool:WaitForChild("Events")
local AttackEvent = Events:WaitForChild("Attack1Event")

local Configurations = Tool.Configurations
local Damage = Configurations.Damage.Value

local SlashFolder = game.Workspace.SlashSounds:GetChildren()
local ImapctFolder = game.Workspace.Impacts:GetChildren()

local Blood = game.ServerStorage.Blood

AttackEvent.OnServerEvent:Connect(function(player,AttackAnim)
	local Character = player.Character
	local Humanoid = Character.Humanoid
	local Animator = Humanoid.Animator

	local SlashAnim = Animator:LoadAnimation(AttackAnim)
	SlashAnim:Play(0,1.8,1.8)

	local SlashSound = SlashFolder[math.random(1,#SlashFolder)]
	SlashSound:Play()

	local BloodClone = Blood:Clone()

	local HitBox = RayCast:Initialize(Tool,{Character})

	HitBox.OnHit:Connect(function(hit,humanoid)
		humanoid:TakeDamage(Damage)
		local ImpactSound = ImapctFolder[math.random(1,#ImapctFolder)]
		ImpactSound:Play()
		BloodClone.Parent = hit
	end)

	HitBox:HitStart()
	wait(1)
	HitBox:HitStop()
	BloodClone:Destroy()
end)
1 Like
2

You can use module scripts to keep your code more organized: https://education.roblox.com/en-us/resources/intro-to-module-scripts

2 Likes
3

A better way to do it would be instead of using a remote even make use of the Tool.Activated event that will fire server side when a player clicks mouse 1. This way you don’t even need a remote event :slight_smile:

1 Like
4

Thanks, I already knew modulescripts but sometimes I forget they even exist and never use them. Silly me.

5

Also how would I be able to add more skills without repeating variables. Say like not doing this:

local SkillOne = AttackFrame.SkillOne
local BarOne = SkillOne.Bar

local SkillTwo = AttackFrame.SkillTwo
local BarTwo = SkillOne.Bar

Is there a way to stop repeating stuff like that ?

6

When it’s on the server the sword is laggy so i’m not sure about that too

7

I would probably use a dictionary if you want to mitigate use of “repeated” variables. Using your reference above, here’s what I would write:

local skills = {
    {
        skill = AttackFrame.SkillOne,
        bar = SkillOne.Bar
    },
    {
        skill = AttackFrame.SkillTwo,
        bar = SkillOne.Bar
    },
}

where you can reference them (almost in the same manner in psuedocode) via:

skills[1].skill
skills[1].bar
skills[2].skill
skills[2].bar

Hopefully that’ll reduce some clutter. :smile:

1 Like