Can you add temp banning so you can specify a time for the player to be banned
2 Likes
This is planned for the next update!
1 Like
Important hotfix
Warning function had an issue where it would only keep the first warning and override the others with the new one, this has been fixed with this update.
THIS IS AN HOTFIX AND NOT A UPDATE
Thank you so much @screenswitch1
1 Like
This could really use profileservice (module)
As this module simply wraps up datastore call functions profileservice would allow for extremely safe and secure datastore saving and crap preventing those who shouldnt be there from well being there
(it also caches so you don’t end up queueing up like 30 calls if you try checking warnings that much for some reason)
3 Likes
This comment is going to be mostly code review
Im going to just throw out alot of stuff here
and not alot of explaining
Code Simplification
When erroring you don’t need to return the error | return error()
When checking if a variable exists you can use assert() Example:
--old method
if UserId == nil then return error("Parameter UserId is nil") end
--better method
assert(UserId, "Parameter UserId is nil")
--even better method (checks that it is a number)
assert(type(UserId) == "number", "Parameter UserId is invalid")
Now looking further on I’m not sure why you didn’t use self here:
local Warnings = service:viewWarnings(UserId)
--could be
local Warnings = self:viewWarnings(UserId)
I would strongly recommend using self here because you are defining your function with : instead of .
Instead of doing this to default
if Reason == nil then Reason = DefaultReasonBan end
--You can do
Reason = Reason or DefaultReasonBan
With checking if the player is still there before kicking
local Player = Players:GetPlayerByUserId(UserId)
if not Player then return end
Player:Kick(Reason)
--There are two alternatives; I reccomend the second
local Player = Players:GetPlayerByUserId(UserId)
if Player then
Player:Kick(Reason)
end
--Second
local Player = Players:GetPlayerByUserId(UserId)
_ = Player and Player:Kick(Reason)
Explanation for the second here: (I made this : clout)
I am very disapointed in this pcall:
local Key = "-Ban"..UserId
local success, result = pcall(function()
ModerationDatastore:SetAsync(Key,{Reason, Moderator})
end)
if not success then
warn("[ModerationService] Failed to Set DataStore Value for "..UserId..".\nError:",result)
return
end
You should really really really really really do it like this:
xpcall(ModerationDatastore.SetAsync,
function(...)
warn("[ModerationService] Failed to Set DataStore Value for ",UserId,".\n\tError:", ...
end, ModerationDatastore, Key, {Reason, Moderator}
)
Time to throw more sources at you
https://create.roblox.com/docs/scripting/luau/functions#variadic-functions
I am going slightly insane looking at your code.
on :CheckBan
i uh
Did you know?
that
you can return multiple arguments:
--from
return {false,false}
--to:
return false, false
--thank you
Instead of concatinating stuff on warn functions like this:
warn("you did bad things" .. player.UserId)
You can do:
warn("you did good things", player.UserId)
its just so much cleaner
Did you know:
tables can have indexes;
local TheGreatestTableAlive = {}
TheGreatestTableAlive.Banned = false
TheGreatestTableAlive.BanReason = "you did something"
it will make your code much more readable, and give developers interacting with your api a good time.
Using this new knowledge:
Apply this to every part of the module; thank you.
After that; I’ll inspect further into the module.
If you need clarification on anything I have said so far, I will be happy to help because I wrote this is five seconds and then did not read it and then clicked the post button and then hoped you would read this and then hoped something spectaculous would happen.
ps. I like the module.
4 Likes
The old method is safer as using assert can lead to memory leaks and can slow down your process, here is a forum explaining this. So I would simply make your own assert function like this
local function Assert(condition, func)
if not condition then
error(func(), 2)
end
return condition
end
3 Likes
People write code to their preference, not everyone would write like how you would. You make some good points, but if you think in the performance aspect, this somewhat makes almost to no difference. You also suggested some stuff that does have some performance cost, such as assert, with ~10-20% slower speeds, and indexed table rather than array tables with ~2-8% speed difference.
I do agree with the indexed table though, it should be more easier for the developers to understand it.
3 Likes
Unfortunate as I was just about to release my own take on something like this. Regardless, please don’t use colons for cosmetic purposes, it can create issues and this just isn’t a use case for a colon.
When you assign a function using a colon roblox assumes it is a method and will provide a self variable, if you call it normally (using a period) because it has nothing to do with objects or OOP as a whole, the first parameter will automatically be assigned to a non-existent self and will just, not work.
local module = {}
function module:doSomething(string)
print(string)
end
module:doSomething("Hello, World!")
-- Hello, World!
module.doSomething("Hello, World!")
-- nil
TL;DR
Don’t create methods unless you are going to use self.
You can read more on methods here
Edit: Alternatively you could just use self as outlined in @Gooncreeper’s post but I’d recommend just using normal functions as this has nothing to do with OOP
1 Like
Thank you everyone for your feedback! I will definitely be using some of your suggestions for the new update.
This seems interesting and useful. I might definitely use this one day.
As for my feedback, I think everything that I’ve noticed as already been said. Although instead of
if not x then ... end
you could do
assert(x, ...)
1 Like
assert has sightly worse proformance
but it straight up ends the code unless your doing something weird it shouldnt matter
1 Like
It’s just my preference, he doesn’t have to if he doesn’t wanna.
1 Like
Hello everyone thank you for your feedback. I will be definitely utilizing them In the next update! Although some of you say that assert gives bad performance, I am not sure if its true, Please answer this poll so I know if i should use it.
Should I use assert
- Yes
- No
0 voters
Btw, what features would you like seeing?
Honestly I dont see the point. You can already easily do this with more control and customizability.
The module might be useless, but hey, It exists and can be used for quick access to functions such as Ban, Warning and such and next update will also come with Time Bans! The code is open source so you can customize this as much as you want.
I mean, it has a good concept but it’s limiting and only has certain use case of banning etc. Some nice features would be the ability to see all users who are banned, temp banning, and another nice feature would be the ability to like use your own custom datastore in order to store the bans.
1 Like
Ill take that in consideration temp/time bans are coming next update, I also add a banlist! I’ll see what I can do about custom DB
1 Like
It’s quite funny to see the spike in moderation modules since WatchDog was released. Nobody has original ideas these days.
Also why include kick if it already exists. It’s just a single line wrapper.
Whats watchdog? Hm? (Pm cause i dont wanna take it off topic)
The kick allows cross server kicking aswell.
1 Like