Implement Database Synchronization Function for Game Playtime #586
Conversation
untested lollmao
This took waaaay too long for me - Switched to Libsql.Client not prod ready? fine by me - Fixed config not loading due to my stupid ass putting the bracket - sphagget over at DBHandler.cs, yum!
- SQL injection fixes - Reinitialize DB instance when its stream expired - Retry operation when error happens - Use UserID hash as the table name
Practically force the data in DB to be updated when user clicking either reset or update playtime
bagusnl
requested review from
neon-nyan and
gablm
and removed request for
neon-nyan
bagusnl
added
Enhancement
Qodana for .NETIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at [email protected]
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
The core functionality seems to be working fine.
However, there are some problems related to error handling and file initialisation when running Collapse for the first time.
Some Front-end related suggestions
- Allow DB Url to not contain
https://or have a different protocol fromhttps://
As the database link copied from Turso is in the format libsql://example.turso.io, it would be nice to be able to input the url directly like that.
- Show a dialog after closing game is syncing wasn't successful.
- Show a dialog if database connection is unsuccessful at start-up.
CollapseLauncher/Classes/GameManagement/GamePlaytime/RegistryClass/CollapsePlaytime.cs
Outdated
Show resolved
Hide resolved
CollapseLauncher/Classes/GameManagement/GamePlaytime/RegistryClass/CollapsePlaytime.cs
Show resolved
Hide resolved
- Early return if DB function is disabled - Put all the props init in the try-catch to prevent stopping error - Not disposing _enabled to reduce amount of calls to config - Removed test code
also random comments here and there
Removed unnecessary database query usage smol optimization bby Also prevent pull function to crash the app by returning Task (learnt my lesson)
CollapseLauncher/Classes/GameManagement/GamePlaytime/RegistryClass/CollapsePlaytime.cs
Fixed
Show fixed
Hide fixed
by discarding the return value, HUEHUEHUE
why did i put it THERE
|
@CollapseLauncher/core-maintainers please finish review in 12 hours if you can, planning to merge this PR on the weekend |
CollapseLauncher/Classes/GameManagement/GamePlaytime/RegistryClass/CollapsePlaytime.cs
Show resolved
Hide resolved
| } | ||
| catch (Exception e) | ||
| { | ||
| LogWriteLine($"[DBHandler::Init] Error when (re)initializing database system!\r\n{e}", LogType.Error, true); |
There was a problem hiding this comment.
Not sure how pertinent it would be to show user-facing error/warning in the form of a dialog but could be good.
There was a problem hiding this comment.
This error will never be displayed as public because I want to set the database system to be completely in the background and non-invasive to the UX. I tried to do push in-app notifications but it seems currently the caller for it doesn't work outside HomePage methods (maybe i am dum, probably, most likely)
There was a problem hiding this comment.
Functionality wise everything seems to be working apart from statistic syncing.
I agree with cryo that could be good to show more errors in the UI when they occur.
Related to changing the url/token/uid in the settings, I believe it would be better to have a "save & validate" button instead of instantly changing the fields (or showing some dialog if first connection fails)
As it is, it is possible to insert a incorrect url and/or token and no error would ever be displayed if the user never clicks the "Validate Settings" button.
CollapseLauncher/Classes/GameManagement/GamePlaytime/RegistryClass/CollapsePlaytime.cs
Show resolved
Hide resolved
This commit allows user to input any string as User ID, but still allows and auto generates GUIDv7 as User ID when needed This also adds a warning badge next to save and validate button, notifying user to save the config when any fields has been changed
|
After discussing with neon in chat, I'm merging this PR. Should there be any concern of the code, hit me up in this PR comment or directly to me |
Main Goal
Synchronize user game playtime to a database that the user can sync with other systems with privacy in mind.
Every user needs to have their own SQLite server (eg. Turso https://turso.tech/ free account should be sufficient) then supply all the necessary informations to Collapse (Database URL in https and the token). Collapse will generate the GUIDv7 as a user identifier table inside the database.
Caution
Changing User ID without taking notes of your current ID will cause the data to be dangling in the database and not being used anymore.
While you can edit the database yourself, it's a bit tedious, so please do NOT lose your user ID.
Playtime synced to the database in a few mechanism:
PR Status :