Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DatabaseReplayProvider #400

Merged
merged 74 commits into from
Nov 19, 2023
Merged

Add DatabaseReplayProvider #400

merged 74 commits into from
Nov 19, 2023

Conversation

vinceau
Copy link
Member

@vinceau vinceau commented Oct 28, 2023

Description

This PR aims to finally address #178 by adding the foundation for a database in SQLite3 to store replay information. It uses the implementation better-sqlite3 and the query builder kysely. This PR has been made possible by the contributors of the past works in #364 #383 #384. Many thanks to those involved.

Notable Changes

Run jest via Electron

Since better-sqlite3 is a native node module, we need to update the command used to run tests. See this comment to know why. This allows us to run the jest tests with the same node version as Electron.

Migrations Handling

We need to package all schema migrations into the package itself for it to be run by kysely on the user's machine. To do this:

  1. We compile the Typescript migrations into Javascript with the new yarn run build:migrations command, with the process defined in webpack.config.migrations.prod.ts
  2. We copy the release/app/dist/migrations/*.js files into the extraResources folder, see electron-builder.json
  3. If the app is packaged, we execute the migrations from the extraResources/migrations folder, otherwise we run it directly from the src/database/migrations folder. See the create_database.ts file for the implementation.

Remaining tasks

  • Better packaging of migration files
  • Somehow move all database handling into a worker
  • Infer game start time before inserting into the database, rather than after
  • Add more fields like set info, and unique player id
  • Add option to re-index the whole DB from scratch. We likely want this when we make major changes to the schema.

@vinceau vinceau changed the title Add database replay provider Add DatabaseReplayProvider Oct 29, 2023
@vinceau vinceau mentioned this pull request Oct 29, 2023
Base automatically changed from feat/replay-provider-2 to main October 30, 2023 00:30
@vinceau vinceau force-pushed the feat/database-replay-provider-2 branch from 757face to 469f333 Compare October 30, 2023 00:34
@vinceau vinceau marked this pull request as ready for review October 30, 2023 09:35
@vinceau
Copy link
Member Author

vinceau commented Nov 2, 2023

@alvarosevilla95 @agiera Do you guys have any thoughts or comments on this PR and the database schema?

Copy link

@alvarosevilla95 alvarosevilla95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @vinceau, been a while! Awesome to see this getting added.

The PR looks fantastic tbh. I really like all the decisions on the schema and db manteinance. I wasn't familiar with kysely before, but it looks great honestly. The typings and the query builders are cool, and so is the migration support it seems. Not much to comment on the schema itself, it looks very reasonable (although you know I'd love to eventually add game stats to the schema :D), and the indexing happens very fast even on a large folder.

I only noticed one small slow point. Fetching players by gameId one by one takes about 4 seconds on my 10k games, which is visible when clicking Refresh. Changing the code to do the join in the db using 'folder' speeds it up to 200ms. I pushed a commit to alvarosevilla95@c9dccc1 if you want to cherry-pick

Great stuff, looking forward to seeing this live!

@vinceau vinceau force-pushed the feat/database-replay-provider-2 branch from 9364ca9 to 43141c3 Compare November 8, 2023 00:18
@vinceau vinceau merged commit f3a6307 into main Nov 19, 2023
6 checks passed
@vinceau vinceau deleted the feat/database-replay-provider-2 branch November 19, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants