Is there an efficient way to rewrite this code?

Hi, it’s my first time trying to make a fighting game with skill moves so I just wanted to know if this code is the best way I can execute with. If not, can someone please give me suggestions on how I should clean my messy code :face_with_open_eyes_and_hand_over_mouth:

local UserInputService = game:GetService("UserInputService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")

local RunService = game:GetService("RunService")

local abilities = script.Parent:WaitForChild("Abilities")

local skill1 = abilities.Skill1
local skill2 = abilities.Skill2
local skill3 = abilities.Skill3
local skill4 = abilities.Skill4

local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()

if character:WaitForChild("Humanoid").Health == 0 then return end

local currentSkill = character:WaitForChild("Humanoid").Skill

local sk1 = false
local sk2 = false
local sk3 = false
local sk4 = false

UserInputService.InputBegan:Connect(function(key)
	--//SKILL 1
	if key.KeyCode == Enum.KeyCode.One then
		if sk1 == false then
			sk1 = true
			sk2 = false
			sk3 = false
			sk4 = false
			skill1.UIStroke.Color = Color3.fromRGB(255, 218, 5)
			skill2.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill3.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill4.UIStroke.Color = Color3.fromRGB(0, 0, 0)
		end
	end
	--//SKILL 2
	if key.KeyCode == Enum.KeyCode.Two then
		if sk2 == false then
			sk2 = true
			sk1 = false
			sk3 = false
			sk4 = false
			skill2.UIStroke.Color = Color3.fromRGB(255, 218, 5)
			skill1.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill3.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill4.UIStroke.Color = Color3.fromRGB(0, 0, 0)
		end
	end
	--//SKILL 3
	if key.KeyCode == Enum.KeyCode.Three then
		if sk3 == false then
			sk3 = true
			sk2 = false
			sk1 = false
			sk4 = false
			skill3.UIStroke.Color = Color3.fromRGB(255, 218, 5)
			skill2.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill1.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill4.UIStroke.Color = Color3.fromRGB(0, 0, 0)
		end
	end
	--//SKILL 4
	if key.KeyCode == Enum.KeyCode.Four then
		if sk4 == false then
			sk4 = true
			sk2 = false
			sk3 = false
			sk1 = false
			skill4.UIStroke.Color = Color3.fromRGB(255, 218, 5)
			skill2.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill3.UIStroke.Color = Color3.fromRGB(0, 0, 0)
			skill1.UIStroke.Color = Color3.fromRGB(0, 0, 0)
		end
	end
end)

To give an overview on how the code is supposed to work in game, whenever the player presses 1,2,3,4 button on the keyboard, they would equip a skill which will be ready to use. What I want is that they cannot use 2 or more of the same skills at the same time. So to do this, I made it so that there are booleans that can be set to false or true (vise-versa) if they use another skill prior to equipping a skill, it would only equip that recent skill.

5 Likes

I feel like i can improve this code, but idk how

See if this works.

local UserInputService = game:GetService("UserInputService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local TweenService = game:GetService("TweenService")
local RunService = game:GetService("RunService")

local abilities = script.Parent:WaitForChild("Abilities")
local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()

if character:WaitForChild("Humanoid").Health == 0 then return end

local currentSkill = character:WaitForChild("Humanoid").Skill

local skillButtons = {
    Enum.KeyCode.One,
    Enum.KeyCode.Two,
    Enum.KeyCode.Three,
    Enum.KeyCode.Four
}

local currentSkillIndex = 0

local function activateSkill(skillIndex)
    for i, skillButton in ipairs(skillButtons) do
        local isActivated = (i == skillIndex)
        abilities["Skill" .. i].UIStroke.Color = isActivated and Color3.fromRGB(255, 218, 5) or Color3.fromRGB(0, 0, 0)
    end
end

UserInputService.InputBegan:Connect(function(key)
    for i, skillButton in ipairs(skillButtons) do
        if key.KeyCode == skillButton then
            if currentSkillIndex ~= i then
                currentSkillIndex = i
                activateSkill(currentSkillIndex)
            end
            break
        end
    end
end)
1 Like

If there are repetitions, the best practice would be to assign them in a table/list/dictionary/etc.
Say you have variables:

local skill1 = 10
local skill2 = 20
local skill3 = 30

print(skill1)
print(skill2)
print(skill3)

We could instead do:

local skills = {
	["1"] = 10,
	["2"] = 20,
	["3"] = 30
}

for key, value in skills do
	print(value)
end
1 Like

What do you mean by this? And yeah I’m getting repetitions.

Whenever I press an input it prints all the 4 skills at once.

image_2024-01-14_142344075

This is the new revised code:

local skillButtons = {
	Enum.KeyCode.One,
	Enum.KeyCode.Two,
	Enum.KeyCode.Three,
	Enum.KeyCode.Four,
}

local currentSkillIndex = 0

local function activateSkill(skillIndex)
	for i, skillButton in ipairs(skillButtons) do
		local isActivated = (i == skillIndex)
		abilities["Skill" .. i].UIStroke.Color = isActivated and Color3.fromRGB(255, 218, 5) or Color3.fromRGB(0, 0, 0)
		print("Skill" .. i)
	end
end

UserInputService.InputBegan:Connect(function(key)
	for i, skillButton in ipairs(skillButtons) do
		if key.KeyCode == skillButton then
			if currentSkillIndex ~= i then
				currentSkillIndex = i
				activateSkill(currentSkillIndex)
			elseif currentSkillIndex == i then
				currentSkillIndex = 0
				activateSkill(currentSkillIndex)
			end
			break
		end
	end
end)
local UserInputService = game:GetService("UserInputService")
local abilities = script.Parent:WaitForChild("Abilities")
local skillButtons = {Enum.KeyCode.One, Enum.KeyCode.Two,
	Enum.KeyCode.Three, Enum.KeyCode.Four}
local skillUIStrokes = {
	abilities.Skill1.UIStroke, abilities.Skill2.UIStroke,
	abilities.Skill3.UIStroke, abilities.Skill4.UIStroke}

local skIndex = 0
UserInputService.InputBegan:Connect(function(key)
	local index = table.find(skillButtons, key.KeyCode)
	if index then skIndex = index
		for i, stroke in ipairs(skillUIStrokes) do
			stroke.Color = i == skIndex and
				Color3.fromRGB(255, 218, 5) or Color3.fromRGB(0, 0, 0)
		end
	end
end)