Kampfkarren / Roblox

Scripts and stuff I wrote for Roblox. Documentation is little to none as these are just stuff I took from my game that I thought I could share.

https://kampfkarren.github.io/Roblox/

Other

284 stars 78 forks source link

feature/GDPR-Tags: Added additional Datastore 2.0 GDPR tag support #166

Closed 4x8Matrix closed 1 year ago

4x8Matrix commented 1 year ago

Context

The Issue https://github.com/Kampfkarren/Roblox/issues/143 was opened but never addressed, this PR is an attempt at addressing this issue.

This PR

Additional call for :GetAsync to implement the same behavior as :UpdateAsync
Updated SavingMethods/Standard.lua to support the userIds array Datastore 2.0 provides.

Kampfkarren commented 1 year ago

UpdateAsync is specifically chosen here because it has resilience behaviors that GetAsync does not provide. Is it not possible to use GDPR parameters with SetAsync here?

4x8Matrix commented 1 year ago

GDPR Tags are not supported in :UpdateAsync (they are supported in :SetAsync), I understand the concern for this to be at production level however this is also quite a big feature which would be really good to get implemented.

This is the source ~ and the reason why I call :GetAsync previous to :SetAsync https://create.roblox.com/docs/scripting/data/data-stores#set-vs-update

would there be any specific way to articulate this same behavior seen in UpdateAsync?

Return operation if the Data is nil
Potentially yield between Get calls to ensure we're not handed a Cache'd version of Data?
Potentially look into Datastore versioning, we can use :ListVersionsAsync and see what's a viable option?

Kampfkarren commented 1 year ago

We can never be confident that a key isn't currently attempting to be updated by another server. This could happen because of someone misusing the DataStore2 API (which sucks, but would suck more to break existing games), or data stores being temporarily down and the server retrying while someone else is in game. I'm not confident the current behavior is very great but using UpdateAsync over SetAsync is somewhat common.

I have an alternate suggestion. Can we make it so if the initial get returns nil, then we use SetAsync for the first time, otherwise UpdateAsync?

4x8Matrix commented 1 year ago

That's a viable solution!

However, i'm a bit worried about calling the :GetAsync now then ~ wouldn't this build up the Datastore get request queue? Essentially we're making two get requests?

Kampfkarren commented 1 year ago

You wouldn't need to call GetAsync here, we perform an initial get as part of DataStore2

4x8Matrix commented 1 year ago

Hey 👋

I've updated this PR with two new commits, 9dfe64b being the additional changes mentioned above (let me know if there's any worry about it's implementation!)

And, e1f90af should help to also help Roblox to tag player-specific data

4x8Matrix commented 1 year ago

Wrong implementation for GDPR Tags, will be creating a new PR soon refactoring this approach to correctly assign a series of UserIDs.