ModerationService - Simplyifying Moderation

[dutycall11 (SavageMode)]

Can you add temp banning so you can specify a time for the player to be banned

[CodedJer (Jeremy)]

This is planned for the next update!

[CodedJer (Jeremy)]

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

[Carrotoplia (Carrotopliani)]

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)

[Gooncreeper]

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 .

What is the difference between defining functions using : and

: is used when the method passes self as its first parameter. In Luau, Roblox forces self as a first parameter. That is how self is recognized. . is what you will use most commonly. ex. Object:func(arg, arg2) is the same as Object.func(Object, arg,arg2)

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)

Using and / or to simplify your code

I figured this out a while ago about the keywords and / or and I have found myself using it all the time to make better code; so today I have decided to spread the knowledge. You probably haven’t thought that much about how and / or work but they actually work in a cool way. I think explaining would be confusing; but coding would be simple so here is a code example of how they work. --assume paramaters are like | a and b | a or b | function And(a, b) if a then return b else return a e…

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

Pcalls - When and how to use them

Intoduction Protected calls, formally known as pcalls, can run functions without erroring and provide if your code was successfully ran but what’s the best way to use them and when should I? In this tutorial I will be covering the entirety of pcalls and where you’ll need them, so lets begin! Pcall basics First and foremost, we should learn what pcall’s syntax really looks like: pcall(f : function, ...args : any) Basically, you pass your desired function in as argument 1 and any arguments 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.

[shr2mp (frames)]

@Gooncreeper

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")

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

[MemoryAddress0 (Memory)]

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.

[Chromatype (chroma)]

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

[CodedJer (Jeremy)]

Thank you everyone for your feedback! I will definitely be using some of your suggestions for the new update.

[zCrxtix (zavier)]

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, ...)

[Carrotoplia (Carrotopliani)]

assert has sightly worse proformance

but it straight up ends the code unless your doing something weird it shouldnt matter

[zCrxtix (zavier)]

It’s just my preference, he doesn’t have to if he doesn’t wanna.

[CodedJer (Jeremy)]

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

Btw, what features would you like seeing?

[TheH0meLands]

Honestly I dont see the point. You can already easily do this with more control and customizability.

[CodedJer (Jeremy)]

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.

[TheH0meLands]

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.

[CodedJer (Jeremy)]

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

[2jammers]

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.

[CodedJer (Jeremy)]

Whats watchdog? Hm? (Pm cause i dont wanna take it off topic)

[CodedJer (Jeremy)]

The kick allows cross server kicking aswell.