Damage Counter For my Game

DamageCounterTest.rbxl (91.3 KB)
(For the code I want to show, go to StarterGui/ScreenGui/LocalScript

Provide an overview of:

  • What does the code do and what are you not satisfied with?

    • My script is a Damage Counter script that shows how much damage you deal to an enemy. It includes a time limit and a check if you’re hitting the same character. I am unsatisfied because it feels like the script can be improved, but I don’t know how.
  • What potential improvements have you considered?

    • None so far
  • How (specifically) do you want to improve the code?

    • There might be a better way to make this. So I would like to know how.
local RS = game:GetService("ReplicatedStorage")
local DCE = RS:WaitForChild("DamageCounterEvent")
local Text = script.Parent:WaitForChild("Damage")

local oldvictim = nil
local DamageDone = 0
local timeleft = 0
local Activated = false

local function Timer()
	if Activated == false then
		Activated = true
		while timeleft > 0 do -- This is the clock right?
			wait(1)
			print("Working", timeleft)
			timeleft -= 1 
			if timeleft <= 0 then
				Activated = false
				DamageDone = 0
				Text.TextTransparency = 1
			end
		end
	end
end

DCE.OnClientEvent:Connect(function(damage, victim)
	if oldvictim ~= victim or oldvictim == nil then
		DamageDone = 0
	end
	oldvictim = victim
	DamageDone += damage
	timeleft = 3
	Text.TextTransparency = 0
	Text.Text = DamageDone
	Timer()
end)

I think your implementation is pretty good.
If I was to implement something similar this would be how I would do It:

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

local DamageCounterEvent = ReplicatedStorage:WaitForChild("DamageCounterEvent")

local Text: TextLabel = script.Parent:WaitForChild("Damage")
local oldvictim: Humanoid? = nil
local DamageDone: number = 0
local timeleft: number? = nil

RunService.Stepped:Connect(function(_, delta: number)
	if not timeleft then return end
	
	if timeleft < 0 then
		timeleft = nil
		DamageDone = 0
		Text.TextTransparency = 1
	else
		timeleft -= delta
		Text.Text = DamageDone
		Text.TextTransparency = 0
	end
end)

DamageCounterEvent.OnClientEvent:Connect(function(damage: number, victim: Humanoid)
	if oldvictim ~= victim or oldvictim == nil then
		DamageDone = 0
	end
	
	oldvictim = victim
	DamageDone += damage
	timeleft = 3
end)

Here are my notes:

  1. Descriptive names for variables.
    Instead of using names like “RS” use names like “ReplicatedStorage”.
    I realize this is mostly a preference thing but as you can see in the edits I made, that variable names this short can start to have conflicts fast (RunService and ReplicatedStorage both would fit RS) as a script grows. Also, the more variable you have in a file the more confusing for you and others it can be to remember what all these variables are referring to. Thats not so much a problem for this script since its very brief, but I consider it good practice.

  2. Type annotations
    This is another preference thing, but I have started adding type annotations to all my scripts recently because there are some small benefits. First, when you use type annotations it allows luau to recommend suggestions for you since it know the types of variables and what properties they contain. When you have lots of different tables and custom types, it can be very helpful in speeding up development. Second if you opt in to the “–!strict” mode the type checker will warn you when you aren’t following the type annotations that you’ve provided. This is very helpful in avoiding accidental bugs.

  3. Using the RunService
    Your timer function works well, but the RunService is made for stuff like this. If you use the RunService like this, you can get rid of your Activated variable and pack this information into the timeleft variable. You can also move all the UI specific implementation out of the client event.

Let me know what you think and if you have any questions.
Also, let me know if I’ve completely missed the mark.

2 Likes