I have player moves that are done via remote event. I check for cooldown on the client before firing the remote event. However, since you can’t always trust the client, I have some extra checks on the server. However, what this means is that each move’s remote event has a lot of cooldown checking code, all of which is copy and pasted amongst all the remote events. This is obviously a bad idea, and so my best idea to make it more efficient was to make one server script that is called, runs the same abstracted cooldown checking code, then calls a BINDABLE event for each ability move.
My question is, is this the best way to do it, or is there a better way?
I think you can just move the cooldowns straight into the server-side scripts to take out those checks altogether. You can keep each players’ individual cooldowns separate by using a dictionary to key their UserId to their cooldowns. You can use the PlayerAdded event to add players’ UserId to that dictionary as they join. If it turns out that I’m stupid and PlayerAdded is a client-only event, you can still send the UserId over via a remote event. In this system, you’d probably want to send the player’s UserId through the remote events used to trigger the moves to know which cooldown to reference.
As for being used among multiple scripts, you should use module scripts instead of regular scripts and bindable events. Module scripts are the ones that are meant for code reuse, so I imagine they are more efficient somehow in this scenario.
Basically, cooldowns are definitely something clients shouldn’t have access to at all. Take it out and place it in server-side. And use module scripts if you want to reuse code.
I find this method pretty easy.
Have a simple debounce on client side so server won’t even get requests until cooldown period is over if the player is not hacking.
Then you can add security on the server side.
When server receives a move request have an if statement in first line looking for a marker in a specific location of the character making the request. If the character doesn’t have the marker accept the move request and add a marker to the character. Set that marker you insert in the character added to Debris after x number of secs (cooldown).
I usually stick a bool value called junk in one of their body parts to use as a marker and my if statement looks in that body part by name to see if it’s there.
Probably not the most efficient method but it’s very effective, easy and I find it fine for low volume requests.
1 Like
Well,
In the course of normal gameplay, the client should always work, and the only time a remote event would be called unintentionally would be through an exploiter. If you fail the server check, you get kicked immediately. This is because the client should never fail the cooldown check on the server, because, if they aren’t exploiting, the cooldown check on the client should never fail. Because of this, I don’t want to remove the checks on the client.
I was thinking about doing this, but I wasn’t super familiar with module scripts. Could you offer some general idea of how I would use them here? It may be obvious if I was more familiar, but from what I gather they’re like your own mini libraries/packages?
1 Like
I wouldn’t put that much trust in the architecture of any game engine, including Roblox Studio. Your remote events, for example, could also be unintentionally called if the cooldown on the client-side runs down faster than server-side does. This can be caused by technical limitations, such as lag.
For that reason, I STRONGLY RECOMMEND AGAINST SWINGING THE BAN HAMMER IN THIS SCENARIO. Bans are meant for serious infractions that you can be 100% certain occurred. This system is not reliable enough and you’ll likely end up banning some players who just spammed their keys too fast. Unless you think trying to throw out the next move as soon as possible is a ban-able offense, reconsider it.
That said, I am wrong when I said that you should remove it altogether. As What_Innit pointed out, having client-side cooldowns does have its merits and you can use it as a way to stop players from firing remote-events when the cooldown definitely isn’t ready. Remote events (and functions), as far as I’m concerned, should be fired relatively sparingly.
In conclusion, don’t swing the ban hammer, but do keep the cooldowns for performance reasons. Just make sure that the server is the one deciding whether or not the cooldown is actually over like I suggested in my first reply.
Yes, thinking of them like libraries is a good analogy. Although I’d argue they are so much more important than I can ever hope to put in words.
To start, you’ll want to figure out which side will use it. If only the server-side will use it, place it in ServerScriptService with the other server-side stuff. For client-side, place them anywhere client-side can access. Keep in mind that both sides will get their own version of the module script, so if you place in somewhere like ReplicatedStorage, you shouldn’t expect it to stay the same between server and client-side.
Now to write the module script. They must always return 1 table or function. As such, most module scripts will be written by instantiating a table with functions like so:
local module = {}
function module.foo()
print("Hello world")
end
return module
You can add extra functions by following the same format as the foo function. That is, if you wanted to write a bar function, you’d write function module.bar().
Now to actually use the module script. This is done via the require keyword.
local module = require(moduleScript)
module.foo() -- Will print "Hello world"
An important nuance to remember is that the require keyword is what actually runs the module script. That is, module scripts do not run until they are used as the argument for require. When they are run, they won’t be run again the next time require is called using them. Instead, they’ll just return the already-established functions ready for the other script to use.
Roblox has a guide on this matter. I know I’ve left out a lot of nuances, some of which might’ve been important to mention, so give that guide a read if you get stuck. I’d also like to reiterate the importance of module scripts. I’m sure you know the importance of being able to reuse code and module scripts should be your primary way of doing so.
1 Like
I added a little bit of delay between the two to make sure that hopefully doesn’t happen as often. There is no banning though, it just kicks you. If it turns out the offset isn’t consistent through testing, I’ll remove it then? Unless you’re certain that still wouldn’t do much of anything.
Apologies for the over-reaction. However, getting kicked mid-battle (at least, I’m assuming you’re making a battle game) still sucks. Having an offset is not an elegant solution, because you’re still relying on an assumption rather than the data. I can’t think of anything to get the cooldowns on both sides of the border to be perfectly in sync with each other, so I still recommend getting rid of it.
If you really don’t want to give up that kicking feature, I’d wait for someone else who doesn’t hate getting falsely accused of cheating as much as I do (I say that earnestly and don’t mean to sound passive-aggressive). Either way, the server should stop anything bad from happening as long as you have all the damage calculating on server-side. As long as you make sure the client is not in charge of calculating anything that affects an opponent (eg. damage, area of effect, applying effects), you should be good to go. If you want my two cents, I say the only thing that the client should send is their player ID, but depending on your existing scripts, that may or may not work out.
1 Like
Yeah, I figured the delay idea may have problems for the same reasons you stated. I’m not too familiar with hardware limitations, but wouldn’t it be more likely for the client to lag, not the server, and thus it not be an issue? Am I wrong, or is the problem more that there is still some chance that the server can lag, and thus it still isn’t very consistent?
1 Like
Just because something is less likely to happen doesn’t mean it won’t be an issue. Servers are computers too and can lag for other reasons. Maybe your game becomes so complex that the server needs time to catch up, or the server just trips up the same reason a normal computer would. These may cause fractions of milliseconds of difference, but under this system, it will be enough to kick a player, which I still firmly believe is a terrible experience. There will always be a degree of uncertainty when it comes to timed communication between different scripts. After all, why else would we need coroutine.yield when we have task.wait?
Automating moderator action should be done on more stable variables (eg. how much damage an attack does), but if you do have to deal with something as dynamic as cooldown, you should implement some wiggle room. You can implement this leniency by increasing how many times players cause an infraction or having a range where the difference is acceptable, just to name a few methods. Depending on the style of your game and the move, some of these won’t work. For example, moves that are meant to be spammed probably shouldn’t use that first idea.
All in all, yes, there will always be false positives when trying to automate moderation on something as dynamic as cooldowns. The best I think you can do is to implement some tolerance and understand that you can never fully get rid of the chance of falsely accusing a player of cheating.
Edit: I stand corrected.
If the cooldown is a NumberValue on the server (so if this NumberValue is greater than 0 we can’t use a skill) (same applies for boolvalue or any other value you are using to simulate a cooldown, or anything really)
Client scripts checks if NumberValue == 0 then (runs the code that fires remote event)
There is absolutely nothing that you should worry about here, and if NumberValue is NOT 0 on the server side you can indeed kick the player as it won’t be a false alarm. However, most of the fighting games do not kick the player right away I feel like, you could have a warning system or something of that sort. (after all you are kicking a player who took the time out of their day to find a cheat/or develop one for your game, that’s a dedicated player lol)
I just thought of something. If I’m interpreting this correctly, your suggestion is to have the client request information from the server, then ask the server again to execute the move. At that point, the kick-checking is out of spite. The server already has the data, so I feel like it should just go with executing the move instead of waiting for the client to confirm to execute the move after telling the client that it’s fine to do so. Unless there’s an advantage you had in mind with going this way?
Oh, so instead of waiting the same amount of time and guessing when the cooldown is up, we just give the server direct information about it so it always just knows?
You want client to click a button to execute the skill. you don’t just execute it just because there is no longer a cooldown
Yeah I mean I use attributes for my cooldowns
Player:SetAttribute(“Cooldown”,true)
task.delay(5,function()
Player:SetAttribute(“Cooldown”,false)
end)
On client I can check if this attribute is true or false, the reason you do client check is so you don’t fire a remote event when the skill is on cooldown because then the server has to listen to the remote event and run the code that could easily be avoided
1 Like
Oh, so you’re creating the attribute on the server?
Yeah right now I have it like that