Please only provide constructive criticism! This is a new service owned by Anonymous Systems. This is not a chat command or a user interface panel This module allows you to call: Ban(); Kick(); and other similar functions, allowing you to integrate it into your own chat commands and GUI Panels. This project is open-sourced, which means you can modify it in any way you see fit.
This uses MessagingService, which means you can kick and ban players across servers. Guess what? The module also allows you to ban players who are offline!
A feature you could add would be temp banning people (can already be made using ban & unban functions provided, but a tempban function would make it easier)
I was reading some posts on the dev forum of users wanting roblox to add a service called “BanService” with :Ban() and :Unban() methods, so I decided to make a resource called “BanService” But I wanted to add more such as warnings so I changed it to be “ModerationService”.
As this module is not integrated into any admin system, yes you can kick mods, if the developer is integrating this to an admin panel/chat commands they are the people responsible to integrate anti-mod kicking!
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
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)
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}
)
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.
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
