Overall the code is pretty nice! I like how everything is organized and commented, I do have one criticism related to your pick-axe tool though.
I noticed that you are using a debounce on both the client and the server, I don’t personally think this is all that necessary, I would instead just use the debounce on the server side.
With an extra debounce on the client you avoid sending a lot of unnecessary events, and you can let the player know that their extra clicks are ignored with an effect (without lag).
Remote events already have a rate limit of about 50 KB/sec, and the client and server can easily de-synchronize, so it wouldn’t even really make a difference in most cases.
I also forgot to mention that this won’t even prevent the remotes from being fired constantly, as they can just fire the remote directly instead of calling the function
Then mobile users will burn through their data 50KB/secs faster!
Also remote events always arrive in order, so if one fails to send all others will have to wait A.K.A. those events will lag.
That is why you also have the server-side debounce.
But not debouncing on the client is just a waste of data, internet bandwith and server processing power (even if it’s only a little) and bad user experience
I would use the CollectionService for the pickaxe code, so you don’t have the same script for every player’s pickaxe.
I would also get rid of the Services, Variables, Functions, etc. comments since they are redundant.
You should only add comments where the code isn’t self explanatory (where someone could ask a question about how it works and why you did it a certain way)
I would also use a single script that calls multiple modulescripts, like in many frameworks. Then you wouldn’t need a BindableEvent since you could just require() and call the function directly. (But this is mostly just preference)
And last but not least I would flip your if statements from something like this:
local function MineOre (player, target)
if cooldown[player] ~= true then
cooldown[player] = true
if target and target.Parent then
local character = player.Character
local humanoidRootPart = character.HumanoidRootPart
local distanceFromTarget = (humanoidRootPart.Position - target.Position).Magnitude
if distanceFromTarget <= 10 then
if CollectionService:HasTag(target.Parent, "Ore") then
local ore = target.Parent
local oreData = ore.OreData
local oreHealth = oreData.CurrentHealth
local mineOreSound = oreData.MineOre
mineOreSound:Play()
oreHealth.Value -= 1
mineOreSound.Ended:Connect(function()
if oreHealth.Value <= 0 then
mineOreSound:Play()
oreMined:Fire(ore)
end
end)
end
end
end
task.wait(1.5)
cooldown[player] = nil
end
end
to something like this:
local function MineOre (player, target)
if cooldown[player] == true then
return
end
if not (target and target.Parent) then
return
end
local character = player.Character
local humanoidRootPart = character.HumanoidRootPart
local distanceFromTarget = (humanoidRootPart.Position - target.Position).Magnitude
if distanceFromTarget > 10 then
return
end
if not CollectionService:HasTag(target.Parent, "Ore") then
return
end
-- only enable the cooldown if all checks have passed
cooldown[player] = true
-- wrap in a coroutine so the code afterwards won't be stopped by the wait
coroutine.wrap(function()
task.wait(1.5)
cooldown[player] = nil
end)()
local ore = target.Parent
local oreData = ore.OreData
local oreHealth = oreData.CurrentHealth
local mineOreSound = oreData.MineOre
mineOreSound:Play()
oreHealth.Value -= 1
mineOreSound.Ended:Connect(function()
if oreHealth.Value <= 0 then
mineOreSound:Play()
oreMined:Fire(ore)
end
end)
end
So it’s less of a pyramid and more like a step by step checklist and thus easier to read.
but I actually just noticed that the pickaxe server code actually doesn’t do anything with the pickaxe itself.
So you could just combine the pickaxe code with the ore handler (or move it into ServerScriptService)
Right now if there are 2 players there will be 2 scripts listening to the MineOre event, which means if one player hits a stone once then the MineOre() function will be called twice. And because the 2 scripts don’t share the cooldown variable the second call won’t be debounced. Meaning that each swing will be twice as powerful, and thrice as powerful with 3 players and etc. !!