Hey there, I am working with remote events for a punch tool I am making for a Super Power Training Simulator like game. Today I come here with the question of: Is this code safe, if not how can I improve on it. also If the code here can be done in a more elegant manner, also please share your advice
Ok, here is the code:
Server script, ServerScriptService:
--// Services
local RS = game:GetService("ReplicatedStorage")
local SS = game:GetService("ServerStorage")
--// Folders
local Remotes = RS.Remotes
local Events = Remotes.Events
local ServerEvents = Events.Server:GetDescendants()
local Modules = SS.Modules
local BigNum = require(Modules.BigNum)
--// Variables
local debounces = {}
--// Helper Functions
local function CheckDebounceTime(debounce, cooldown)
return debounce < os.time() - cooldown
end
local function BigNumAdd(stat, amount)
local bNum = BigNum.new(stat.Value)
local result = BigNum.mt.add(bNum, amount)
stat.Value = BigNum.mt.tostring(result)
end
--// Handlers
local RemoteHandlers = {
["Punch"] = function(player)
if debounces[player.UserId] then
local playersDebounces = debounces[player.UserId]
if playersDebounces["Punch"] then
local debounceTime = playersDebounces["Punch"]
if CheckDebounceTime(debounceTime, 0.75) then
playersDebounces["Punch"] = os.time()
local Strength = player.Strength
BigNumAdd(Strength, 1)
end
else
playersDebounces["Punch"] = os.time()
local Strength = player.Strength
BigNumAdd(Strength, 1)
end
else
debounces[player.UserId] = {
["Punch"] = os.time()
}
local Strength = player.Strength
BigNumAdd(Strength, 1)
end
end,
["Endurance"] = function(player)
if debounces[player.UserId] then
local playersDebounces = debounces[player.UserId]
if playersDebounces["Endurance"] then
local debounceTime = playersDebounces["Endurance"]
if CheckDebounceTime(debounceTime, 0.75) then
playersDebounces["Endurance"] = os.time()
local Endurance = player.Endurance
BigNumAdd(Endurance, 1)
end
else
playersDebounces["Endurance"] = os.time()
local Endurance = player.Endurance
BigNumAdd(Endurance, 1)
end
else
debounces[player.UserId] = {
["Endurance"] = os.time()
}
local Endurance = player.Endurance
BigNumAdd(Endurance, 1)
end
end,
["Psychic"] = function(player)
if debounces[player.UserId] then
local playersDebounces = debounces[player.UserId]
if playersDebounces["Psychic"] then
local debounceTime = playersDebounces["Psychic"]
if CheckDebounceTime(debounceTime, 0.75) then
playersDebounces["Psychic"] = os.time()
local Psychic = player.Psychic
BigNumAdd(Psychic, 1)
end
else
playersDebounces["Psychic"] = os.time()
local Psychic = player.Psychic
BigNumAdd(Psychic, 1)
end
else
debounces[player.UserId] = {
["Punch"] = os.time()
}
local Psychic = player.Psychic
BigNumAdd(Psychic, 1)
end
end,
}
--// Connect The Remotes
for _, Event in ServerEvents do
if Event:IsA("Folder") then continue end
Event.OnServerEvent:Connect(RemoteHandlers[Event.Name])
end
Local script in a tool that calls the remote:
--// Remote Folders
local RS = game:GetService("ReplicatedStorage")
local Remotes = RS.Remotes
local RemoteEvents = Remotes.Events
local ServerEvents = RemoteEvents.Server
local ToolEvents = ServerEvents.ToolEvents
--// Remote Events
local PunchEvent = ToolEvents.Punch
--// Variables
local Tool = script.Parent
local Debounce = false
--// Functions
local function OnToolActiaved()
if Debounce == false then
PunchEvent:FireServer()
Debounce = true
task.delay(1, function()
print("Debounce reset")
Debounce = false
end)
end
end
--// Connections
Tool.Activated:Connect(OnToolActiaved)
All the remote events look secure to me. And the script is laid out nicely so you can see everything and understand it! I say it’s good. (Also, yeah Code review might be a more fitting category).
Looks safe to me, since you reject punches outside of the debounce window on the server. An elegance edit you could do, is instead of having two sections of your punch function, one if a debounce exists, and the other if there isn’t one, you could merge them by making your conditional into:
“if debounceTime == nil or CheckDebounceTime(debounceTime, 0.75) then”
Since the following code for both conditions is identical.
good choice to do both server and client debounces
something unrelated I want to point out is that its better to convert the functions to modules instead of putting it in a dictionary, modularity is a good practice when it comes to scripting
One checks if the debounce for a specific tool/remote exists, if not it will run the code and add that debounce, the CheckDebounceTime(debounceTime, 0.75) is something that worries me a little, it’s not about the function it self it’s about the cooldown time. You see, in the local script I already have a debounce for 1 second, and it seems if I check the debounce on the server with also 1 second it takes like around .25 seconds longer than 1 second. So brining it down to .75 made it work appropriately, however, if a client exploits by the client debounce then they will be able to fire the server one faster than they should be, any fixes @Weeve or anyone else.
I am aware. But both the checks could certainly still be combined without any change in function.
And no. 0.25 is an alright window. Preferably, you’d have it as a variable you could adjust, and tune later for what appears to be the actual packet offset, but that there’s some offset allowance is pretty standard networking safety. Exploiters are technically able to gain that 0.25, but most exploiters want to do more than just punch a little faster.
If it is the main source of gameplay, and it will be targeted by exploiters, I like to set a counter for a much larger time window, like over the span of a minute, where it will increment the counter for both passed and rejected uses. Any normal player will only have 1 punch more than the max number of uses that can efficiently pack into that minute. An exploiter will fire that remote dozens of times more than they were supposed to, and in some cases, hundreds more, since the most popular way to exploit things like the 0.25 slop is to just spam the remote as fast as possible.
The above is certainly overkill in most cases, but I have done it a few times before. If you’re going to implement something like this, make a note of it in your code, and ignore it for now. Come back once your game is ready for launch/is popular, and at that stage, start doing all those small tasks you made notes of. Getting a game to a launchable state is such a big important event, that to slow it down for something exploiters may not even be interested in, isn’t worth it.
It definitely is, idk if you have every played any games like super power training simulator, however, the idea is you have to train those stats using those tools to progress through quests, get stronger, and unlock new powers.