Is this good for getting data on the client?

Help and Feedback Code Review
#1

So something I struggle with is sending data to the client/requesting data from the server I don’t think there’s a proper way to do this besides just using remote events, but I feel like the way I currently do it isn’t good lol.

I have a module script in replicated storage that does this

local module = {}

function module.GetData(Player,Data)
	print("Getting Data")
	local Data = GetData:InvokeServer(Player,Data)
	return Data
end

function module.CheckData(Player,ItemType,Item)
	local CheckIfOwned = CheckIfOwned:InvokeServer(Player,ItemType,Item)
	return CheckIfOwned
end

return module

And then in any local script where I need the players data I just do

--| Modules
local RequestData = require(RepStorage.Modules["Request Data"])

--| Variables
local Data

wait(3)

Data = RequestData.GetData()

Is this fine? or is there a nicer/better way to do do this?

#2

You do not need to pass the player as a parameter in RemoteFunction:InvokeServer() as it is automatically the first parameter in the RemoteFunction.OnServerInvoke callback.

#3

Since ReplicatedStorage only replicates to all machines, no changes you make on one of the replicas are visible elsewhere. And that’s great, because clients aren’t able to change data by themselves that way. A better place for your module script would be ServerStorage or inside the server script that manages data. Replication is not necessary, and clients can’t read module scripts that way. Remote functions pass first argument automatically, so you always have to start the receiving function with player parameter. Same logic applies to remote events. Remote functions are alright, but you can probably simplify the process even more. Using Players.PlayerAdded event, you can wait for player to join, wait for them to load, and send them data without the player sending a request.