Code review on my data getting module

local DataStoreService = game:GetService("DataStoreService")
local ServerStorage = game:GetService("ServerStorage")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local PlayerDataStore = DataStoreService:GetDataStore("PlayerDataStore")

local remotes = ReplicatedStorage.Remotes

local module = {}

function module.Get(player, dataFolder, defaultDataTable)

    assert(dataFolder:IsA("Folder"), "Argument 'dataFolder' folder must be a folder")
    assert(player:IsA("Instance"), "Argument 'player' must be an instance")
    assert(typeof(defaultDataTable) == "table", "Argument 'defaultDataTable' must be a table")

    local key = script.Parent.Parent.DataHandling.Name .. "/" .. player.UserId

    local get_Data_Tries = 0
    local maximum_Get_Data_Tries = 5
    local get_Data_Interval = 3

    local data
        
    repeat wait()

        local success, errormessage = pcall(function()
            data = PlayerDataStore:GetAsync(key)
        end)

        if success then
              for _, v in ipairs(dataFolder:GetChildren()) do 
                 if v.Name ~= "DataLoaded" then
                    if not defaultDataTable[v.Name] then -- [v.Name] must exist in the defaultDataTable so it can have a default value if it wasn't saved in the data table
						warn(v.Name .. " was not found in the defaultDataTable")
                    end
                    v.Value = data[v.Name] or defaultDataTable[v.Name]
                 end
             end
             dataFolder.DataLoaded.Value = true
             print(player.Name .. "'s data successfully loaded")
        else
            wait(get_Data_Interval)
            get_Data_Tries += 1   
         end

    until success or get_Data_Tries == maximum_Get_Data_Tries
end

return module
  • This is a module that get’s saved data and loads it in. I want to see what can be improved.

  • I’ve considered some improvements such as checking if the data is nil in the data table. If so, set it to the default data table given ( as the argument ).

  • I want to improve how the module get’s the data, checks efficiently if the data is nil given in the table. ( Though I have already implemented this, do let me know if my method alone is good enough ). The last thing I’d want to improve is by adding in repeat calls just in case the pcall has failed. I need some advice on how would I approach this properly.

1 Like

Im seeing no security or sanity checks which is a security concern. Plus the fact that its a module makes it easily accessible. This goes for you and the exploiters.

Actually getting the data looks decent though other than the fact that its trying to save an array. Datastores are made to save string, so idk how your going about saving the array.

For further improvement:
1.Use a remote function instead of a module. Yes its accessible, but… exploiters can access it easily. The easier your module is to use, the easier it is to exploit.
(Invoke server / client. It requires a returned value so is more secure than plain events)

2.It could do with a mass security upgrade. (Encryption, Sanity checks and debuff for firing events multiple times)

3.My prefered method for saving data is with combined string. You can do this with
table.concat(Table,Seperator) and then string.split(string,Seperator). This means you can quickly save data and reduce the number of requests. This means you will only have to check if one datastore is nil as opposed to multiple which will result in longer waits for fetching data.

It saves a table, not an array.* What do you mean by use a remote function instead of a module? It’s server sided so I am confused on what you mean by that.

Can you give an example of what you mean by saving a data with a combined string?

For example:

local Array = {"String1","String2","String3"}
local Result = table.concat(Array,";Split;")

– Result:
“String1;Split;String2;Split;String3”

and to split it back up:

local String = "String1;Split;String2;Split;String3"
local Result = string.split(String,";Split;")

– Result
{“String1”,“String2”,“String3”}

The module is server sided, exploiters cant view scripts in Server Script Service (I assume its there and it should be), using Remote Functions causes unecessary yield for no good reason and infact an exploiter can remove the client callback causing an infinite yield.

Edit: I’ll also add up that you don’t need exploiter sanity checks as it is being kept and handled on the server. The client has absolutely no control on what gets saved and what not (unless they learn to exploit remote events which give them higher stats but thats a different thing to look into).


This is rather a Data Getting script so theres not really much to do about it.

3 Likes

Yes that will work for some exploiters. But very few.

The majority use premade software that can read local scripts and modules. Putting them in the ServerScriptStorage will not hide it from the majority of exploiters as you can litterally find the most basic of DEX explorers and this will be able to see hiden services, function and scripts within the game. And they can also read the memory so they can see whats going through your modules and local scripts.

I wish that were the case but for the majority of the time it wont do.
The people that develop exploits can see everything, this they use to release a exploit for your game. This is how the majority do it as their exploit wont have that kind of capabilities, only the select few have it, but those who do are the ones that release scripts and exploits for a game.

Roblox doesnt replicate the scripts in Server Only services and its kept with the server always. There’s no way they can get access to it. Your client is all you have when you use exploiting software, you are not hacking into the roblox server but exploiting whats available to you.

1 Like

exactly. and giving them a module with no security is like putting it right into their hands.

I am sorry but do you understand what I mean?

Roblox doesn’t pass or give the scripts to the client in order for them to see them. Its just always kept on the server.

Exploits are softwares injected into the roblox client on your computer and they only have access to what you are able to as a Client, i.e Teleport your character around, exploit remotes.

You are not injecting the exploit you have into the Server, theres no way you can touch that with a simple exploit on your pc.


I concluded that its impossible for them to view your server scripts unless its in a replicated place. And it is impossible to edit and make changes to a server script and replicate it to the server, let it be located anywhere. So its completely safe in this case.

1 Like

server is not editable though client. however info can be passed on via modules / remotes.

Server does not replicate from client. But server is visible to client.
You cannot change a server script.
But you can feed it information by monitoring what is put into it and what comes out.
E.g if I see a module gets fired with “minimic2002” 300 and my cash goes up by 300. What do you think im going to do. Obviously im going to fire it and make myself rich.

Edit:
At no point did I say it was injected into the server. Its manipulated from client side.

Your understanding of modules if wrong. Yes, client can see what remotes get fired and information that is being passed but client can’t view or edit modules that are stored in services that are not replicated to client (SSS,SS) .

1 Like

thats true. but if the local script requires the module it has to be in a reachable location. Meaning its exploitable.

Normal scripts can require it and in this case they do, this is just handler that helps with organization and easier usage in multiple scripts. If you want local script to be able to require it, only then you put it in service that is replicated to client.

3 Likes

if you are doing that its the worse thing ever. The server knows what to do, you dont have to pass it data which it will store all your operations that affect everyone will have to be passed to the server at times, thats the time you sanity check you dont just fire a remote to the server to tell it what to save.

The client has no way to see something that isnt replicated. If you have ever playtested before, you must know that ServerStorage and ServerScriptService appears empty on client. It isn’t being passed to the client for it being able to exploit it. Even if you did reach the module script and require it if its out in the open somewhere, you literally can’t do anything as its required on the client, you cant save on the client, the sanity checks you are talking about also are of no use as they can edit it but do nothing. If you don’t know how modules work, if you require one on the client, it runs client sided, if you do it on server it runs server sided, so don’t expect any :SetAsync() to work. Theres absolutely no way. So this statement is totally invalid.

The server should already have what it needs to save during the gameplay as it gets information and carries out sanity checks, this is not something you bother about when making a datastore script.

Also: Modules can’t be used for Client - Server communications so you can’t monitor what the server is doing if its server sided only.

2 Likes

Server scripts/ and serverstorage contents from the server do not replicate to the client. The client can only view what he/she put in serverstorage/serverscriptservice.

For example a client put’s a part in serverstorage he would only be able to see that he wouldn’t be able to see the contents of serverstorage from the server, since the server doesn’t replicate things from serverstorage and serverscriptservice to the client.

You can’t view things which aren’t replicated to you that makes no sense.
https://developer.roblox.com/en-us/api-reference/class/ServerStorage


https://gyazo.com/b19beaffbed16c906b94364eb2dedc20

Anything not replicated to the client WILL NOT be visible to the client. Sevrerstorage is not replicated to the client.

Anything in serverStorage won’t replicate to the client


https://gyazo.com/b19beaffbed16c906b94364eb2dedc20