Is there any other way to improve my DataStore more?

Help and Feedback Code Review
#13

I see this all the time when people use pcall. There is no need to wrap this in a function as pcall takes a function followed by any arguments to pass to that function.

pcall(function()
	StatsData:SetAsync(Player.UserId,DataStoreModule.SaveData(Player.leaderstats));
end)

You can use

--  you need to pass the table as : is used
pcall(StatsData.SetAsync, StatsData, Player.UserId, DataStoreModule.SaveData(Player.leaderstats))
1 Like
#14

What about this?

pcall(function()
StatsData1:SetAsync(Player.UserId,DataStoreModule.SaveData(Player.leaderstats));
StatsData2:SetAsync(Player.UserId,DataStoreModule.SaveData(Player.leaderstats1));
end)
#15

You would want to use two pcalls as you do not want to duplicate the request if one fails.

2 Likes
#16

So what I’m trying to say is that he should use metatables, I’m not great at them but you should definitely put them in a module.

#17

Metatables are not going to help in this use case. Metatables are not an alternative to ValueObjects, they’re intended to run metamethods on tables for certain cases, including the following cases:

  • If a member of the table is indexed or getting an extended index
  • Deciding what is to be done when a new value is created
  • Determining what should happen if a table variable is called like a function
  • etc.

See the metamethods Developer Hub article for more information on metamethods (since that’s the only real part of metatables you need to know, metatables are as simple as setmetatable(table, metatable)).

A proper alternative to ValueObjects is ModuleScripts, which OP is using aside from the actual registration of data which is loaded out across ValueObjects. Depending on OP’s use case and preferences, this advice could hold no value to them.

2 Likes
#18

Oh ok, that makes sense.

#19

My recommendation is what I usually do: Cache the data in a table. When I write Data Store modules I try to make as little use of “the Async’s” as possible.

#20

So how can i fix that?

#21

Retrying a reasonable amount of times and if it still doesn’t work, leaving them with the default data but:

  1. Telling the player their data is a backup and warn them their data won’t be saved
  2. Not saving it under any circumstance
#22

What if so I made BoolValue called DataLoaded if DataStore throws errors I won’t make the value true if it loads then I will set the value to true. When a player leaves it will check if DataLoaded value is set to true if it is set to true, then it will save the data if not then it won’t save it and the player will keep his old data. How does this sounds?

Why is my datastore not saving?
#23

That sounds fine to me.

1 Like
#24

If a datastore throws error should I kick the player or make message that tells him that the data was not loaded successfully?

or send him to new lobby

#25

That’s up to you, but I would suggest just making sure the player is aware their data didn’t load with the option to server hop.

1 Like
#26

So something like this?

local Load = function(folder,data,plr)
	
	local Succsess,Data = pcall(function()
		return data:GetAsync(plr.UserId);
	end)
	
	if (not Succsess) then
		print("faild to load data");
		return;
	end
	
	if (Data == nil) then
		return;
	end

	plr.DataLoaded.Value = true;
	
	for i,v in next,Data do
		print(v.name)
		if (folder:FindFirstChild(v.name)) then
			folder:FindFirstChild(v.name).Value = v.value;
		else
			print(v.name.." is not valid member of "..folder.Name);
		end
	end
end
#27

You should attempt to retry the GetAsync n times. You should also tell the players their data didn’t load.

#28

Ok, thanks for the help.

1 Like
#29

I am running into an issue now If a player is new to the game and I load data it would be nil and if datastore throws error error it would also be nil how would I detect if the player is new to the game and the data is nil because of that or because the datastore did not load successfully.

local Module = {}

function Module:Load(plr,leaderstats,GDS)
	local Succsess,Data = pcall(function()
		return GDS:GetAsync(plr.UserId);
	end)

	if (Succsess and Data) then
		for i,v in next,Data do
			if (leaderstats:FindFirstChild(Data.name)) then
				leaderstats:FindFirstChild(Data.name).Value = leaderstats:FindFirstChild(Data.name) + v.value;
			else
				warn(v.name.." is not valid member of "..leaderstats.Name);
			end
		end
		plr.DataLoaded.Value = true;
	else
		plr.DataLoaded.Value = false;
	end
end

function Module:Save(leaderstats)
	local Data = {};
	for i,v in next,leaderstats:GetChildren() do
		local TempData = {
			name = v.Name;
			value = v.Value;
		};
		table.insert(Data,TempData);
	end
	return;
end

return Module;
#30

pcall returns whether the function inside errored or not. Just check that value.

1 Like
#31

So like this

function Module:Load(plr,leaderstats,GDS)
	local Succsess,Data = pcall(function()
		return GDS:GetAsync(plr.UserId);
	end)
        if (Succsess and Data == nil) then
          plr.DataLoaded.Value = false;
        end
	if (Succsess and Data) then
                plr.DataLoaded.Value = true;
		for i,v in next,Data do
			if (leaderstats:FindFirstChild(Data.name)) then
				leaderstats:FindFirstChild(Data.name).Value = leaderstats:FindFirstChild(Data.name) + v.value;
			else
				warn(v.name.." is not valid member of "..leaderstats.Name);
			end
		end
	end
end
#32

No. This’ll fire if the player has no data.

Check if it didn’t succeed. If it didn’t succeed, that means it errored.

1 Like