MadStudioRoblox / ProfileService

Universal session-locked savable table API
https://madstudioroblox.github.io/ProfileService/
Apache License 2.0
292 stars 157 forks source link

Use RemoveAsync to wipe profiles #13

Closed colbert2677 closed 3 years ago

colbert2677 commented 3 years ago

Contrary to the note for the profile wiping implementation, RemoveAsync can set a DataStore key to nil. Changed the implementation to use RemoveAsync instead to represent a true data wipe.

For backwards compatibility reasons I did not remove the bit in the main transform function that sets latest_data to nil if the value is PROFILE_WIPED. That being said, for newer games that wipe profiles the DataStore entry will be completely removed instead of having a value of PROFILE_WIPED.

colbert2677 commented 3 years ago

EDIT: Included more info than my original reply. EDIT 2: Links, word cleaning and more!

@SilentsReplacement All DataStore work in ProfileService (via StandardProfileUpdateAsyncDataStore) is already wrapped in a pcall (lines 639-741). RemoveAsync falls within that pcall. The pcall working with DataStores in StandardProfileUpdateAsyncDataStore would return a success/error message based on the UpdateAsync for non-wipes and the RemoveAsync for wipes.

It would be redundant for me to perform a pcall within a pcall. The top-level of the pcall decides, based on if update_settings contains a WipeProfile indice, whether it's going to perform an UpdateAsync to update or set the value of the key to PROFILE_WIPED; I just changed the wipe path to RemoveAsync. If RemoveAsync fails then update_status will not be reached and the pcall will terminate with the error. wipe_status is defaulted to false and in the case of a failed wipe then false will get returned (return value of WipeProfileAsync is is_wipe_successful). In all respects a failed RemoveAsync will be treated as a failed wipe.

Would also like to point out that the code keeps convention except instead of setting the value of the key in the DataStore to PROFILE_WIPED I'm directly removing the data from the DataStore. The original UpdateAsync code verified if the wipe was successful by checking if UpdateAsync returned PROFILE_WIPED; RemoveAsync returns the data saved to the value so the return value can be discarded. The code is pretty much functionally equivalent, just a PROFILE_WIPED value versus a nil value. I guess the one red flag might be figuring out if RemoveAsync validates like UpdateAsync though and doesn't ignore like SetAsync, which would explain the rationale of checking the return value before setting wipe_status.

Appreciate you looking out for me though in case I made a mistake but in this case I did not. 😄

LM-loleris commented 3 years ago

Will postpone this change to the DataStore 1.1 ProfileService update

LM-loleris commented 3 years ago

Feature added! Thanks for your submission!