kampfkarren / roblox Goto Github PK

Easy fix, but a backwards incompatible one. Will have to check if anyone is relying on this behavior, though I can't imagine it.

Solution: Keep a counter of players that goes down on PlayerRemoving, and make the BindToClose yield the event until the player counter is zero.

This means :Set has the potential to yield if you didn't already :Get before hand!

Rotriever does not seem to support monorepos like this, so DataStore2 will have to be given special treatment. Also, .module.lua is an ancient file naming that has been superceded by simply .lua for a while.

Relevant sections are

Inside ServerScriptingService is this script

player.CharacterAdded:Connect(function(character)
	wait()
	local checkpointStore = DataStore2("checkpoints", player)
	local playerCheckpoint = checkpointStore:Get()
	if playerCheckpoint then
		teleportCharacterToCheckpoint(character,playerCheckpoint)
	end
end)

function teleportCharacterToCheckpoint (character,checkpointNumber)
print("Teleporting to "..tostring(checkpointNumber))
local checkpointPart = CheckpointManager:findCheckpointNumber(checkpointNumber) 
if checkpointPart then
	
	teleportCharacterToCFrame(character,checkpointPart.CFrame)
end
end

everything was working fine until datastore2, which now seems to be losing the initial get...

A .OnFail for DataStor2 so if it fails to save data you can do something such as notifying the client.

Perhaps DataStore2.FromUserId?

Solves the use case for #47.

When a player leaves and their data isn't set, then data doesn't save. However, if you set their data and save twice, it'll truly save twice.

Would save the data of every data store the player uses.

Currently, it's unintuitive that saving one data store in a combined data store saves all of them. This would be the canonical way to save data when using combined data stores (default in #38).

Haven't tested yet, but I can guarantee that https://github.com/Kampfkarren/Roblox/pull/33/files broke unit tests (not the authors fault!) as the tests use a table to mock players. This isn't that hard of a fix, it's just an annoying one.

DataStore2 is, at its core, a generic and more easy to use data store API that caches. I'd like everyone to use DataStore2.

However, the saving method of DataStore2 prevents this vision. Furthermore, if Roblox ever does anything to prevent this saving method, developers are at a complete loss.

Thus, a generic internal data store saving API should be added, and offer both the berezaa method and normal data store saving. Berezaa should be the default unless anything changes in the future.

This should NOT break any games, and nobody should have to change anything.

The entire verifier code was adapted from Coercii's code here. There should be a comment giving them the credit they deserve.

No known repro case, it's happening in my game where:

local store = DataStore2("someCombinedKey", player)
store:BeforeInitialGet(callback)
print(store:Get()) -- The result of BeforeInitialGet
print(store:Get()) -- The original data?

BeforeInitialGet is only ran once, as observed by a print inside.

Right now it just tells you a warning without a stack trace, which seems weird.

Use case: You want to teleport people to a new reserved server. You want to make sure you can get their data in the other server, so you :Save() before teleporting.

When they teleport away, they leave and their data is redundantly saved.

Alternatively, this could also be solved by correctly not resaving data if it hasn't been changed.

https://github.com/osyrisrblx/t

I've never tested it and it doesn't even work, it tries to save the data store even if the player already left.

Promise.async always adds a non-blocking yield--we should avoid it wherever possible.

Possibly reliant on LPGhatguy/lemur#85, but I can get away with just implementing it myself.

:Save() does not pcall, but it seems strange for :Get() to pcall if this isn't the case.

An API like :SetBackup should be introduced for :Save() as well.

:SetBackup is fine right now, but leaves a weird future. Using it with different values on combined data stores will only pick one of them. (An issue when combined data stores will eventually be the default).

#38 will need a global configuration, so might as well put this in there as a key that affects all data stores (like :SetBackup does already with combined data stores) on how many tries it'll take before a backup is used instead. For anyone who will explicitly opt out of combined data stores, :SetBackup should continue to work fine and should not give a deprecation warning.

The way Mkdocs is generating the basic usage page, the list is not rendered as a list.

If you check the file on github it does render correctly but mkdocs does not understand that it's a list

Would be the same as Save() only it saves in a spawn coroutine block.

os.time has inconsistency across servers. An incrementing counter would be better, but is a bit difficult to do as it'd need to have context of the old data store?

This is arguably critical?

List games that use DataStore2, etc.

"Roblox" is confusing, and it would allow me to easily just have a .d.ts file for roblox-ts.

The value is whatever the data store is, but it still shouldn't call.

Ideally, this'd be something in Studio. Don't want to make people worry about GDPR.

I'm pretty sure cancelling a promise will give you a strange error, so make sure it doesn't.

Okay, I don't expect you to do this as DataStore2 is already awesome. But it would be amazing to see a DataStore3 with berezaa's new system. You can read exactly what he said [here].

Reference image:
(https://twitter.com/berezaagames/status/1122174490882998272)

I would love to see you implement this. If you did I would recommend just strictly putting it into DataStore2 but leaving DataStore3 options under a "beta" flag and it will be completely optional and eventually be forced?

This is the main thing PlayerDataStore (or whatever it's called, I just learned it about it today hehe) has over DataStore2. It saves often, and makes sure to be smart and wait 10ish seconds before saving again (the same write key limit). I want this! I have regular save off in every game and only made it for anyone that'd want it, but if I had this it would make my life so much easier (worried about people's data not saving in Battle Hats sooo bad).

In theory, this line prevents any errors when saving data. In practice, the :Save() function has its own pcall so this function never fails.

Make it so that you can easily load a backup onto a main datastores or even if the player completely loses data it fully loads the backup and if there isn't a backup it goes through each old key until one of them works and loads that one. In the chance everything is lost how exactly can data be retrieved because it takes a lot about how this can prevent data loss but doesn't say much about giving a user their data back.
Also allowing us to use the MasterKey as a normal DataStore would be a great feature and using it with :OnUpdate() I feel as if OnUpdate should return the key saving as well. Here's an example:

DataStore2.Combine("MyMasterKey", "Coins", "Gems")

local Coins = DataStore2("Coins", Player)
local Gems = DataStore2("Gems", Player)

-- Maybe have it as a variable in DataStore2???
DataStore2.MasterKey:OnUpdate(function(SavingKey, Value)
       print(SavingKey, Value) -- If coins saved with value 100 it'd print Coins, 100
end)

Coins:Increment(100, 0)
-- etc.

-- The idea of using MasterKey as a normal datastore:
local Master = DataStore2("MasterKey", Player)

print(Master:Get().Coins) -- Use it like a table.

What I sent you on DevForum for reference:

This would prevent a lot of data bugs from reaching production.

Minor memory leak

Would take in a function, and every time you call :Set/:Update/etc, the validator is checked and errors if the data is invalid.

Similar to Roact, would be off by default, but be recommended for development.

All too often I don't catch myself doing DataStore2(player, "name") instead of the other way around and DataStore2 doesn't stop me, it just gives a vague error.

Say you had the code:

dataStore:Get(10)

If the player had no data, they'd get "10". Say a player leaves after this and :Set is never called. This data won't save. This doesn't really matter until you change the default value, say:

dataStore:Get(20)

Now all players who used to have 10 and whom :Set was never called will now get 20.

This is a niche edge case that probably doesn't matter all too much, but it's surprising.

I want to run luacheck on PRs (once I make sure nothing actually has any warnings), Travis, and a code coverage tool.

I want the recommended way to use the module going forward to be to either get the free model (once I can actually update it again, and hopefully also use packages) or to download the releases on GitHub.

People requiring by ID (which judging by the survey is a LOT) prevents me from making any backwards incompatible changes.

After doing some testing, I think what's happening is when a game server is shutdown, the player's data is reverted back to the data received from the initial :Get() when the player joined the server, ignoring any changes made to the player's data while in the server.

This is something that wasn't born until after it was seen to be an issue without, but new developers won't realize how necessary this is and not do it. Perhaps introduce DataStore2.Uncombine for the opposite, or just tell developers that were using DataStore2 before to just disable the settings. Also introduce a setting for a master key.

I'm unsure how I want it to actually act in the case of an error. Retry based on the number of retries set then log an error?

The following flow:

- Get() the data store, the value is { foo = "bar" }
- Set({ foo = "bar" })
- Save()

...results in a superfluous save.

Instead, Save() should check the previous data to see if it is identical.