-
Notifications
You must be signed in to change notification settings - Fork 20
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 scaffolding for PRC-1 achievements API #358
Conversation
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
"build": "tsc --build tsconfig.build.json", | ||
"prebuild": "npm run compile:api", | ||
"compile:api": "npx tsoa spec-and-routes" | ||
"build": "npm run compile:api && tsc --build tsconfig.build.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to move this out of the prebuild?
I don't remember exactly why it was here, but I think it may have to be with the fact that putting it in the prebuild ensures that it gets run before the build step of any package in paima-engine (so any package can access the generated types). I don't think this is used in practice though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I ran into is that if this stays in prebuild, tsoa can't import the PRC-1 types from utils-backend
on clean builds, or otherwise sees the old definitions, because they're produced on that package's build.
Maybe can somehow be improved by teaching NX to build utils-backend before prebuilding paima-rest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NX should be able to figure out dependencies based on the references
inside the tsconfig IIRC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to not work. It doesn't infer that rest:prebuild depends on utils-backend:build
~/paima-engine (patch/standard-achievements-api|✚3) $ rm -r packages/node-sdk/paima-utils-backend/build/
~/paima-engine (patch/standard-achievements-api|✚3) $ npm run build
> @paima/[email protected] prebuild
> npx nx run-many --parallel=${NX_PARALLEL:-3} -t prebuild
✔ nx run @paima/utils:prebuild (2s)
✖ nx run @paima/rest:prebuild
> @paima/[email protected] prebuild
> npm run compile:api
> @paima/[email protected] compile:api
> npx tsoa spec-and-routes
Generate routes error.
GenerateMetadataError: No declarations found for referenced type AchievementPublicList.
at TypeResolver.getModelTypeDeclarations (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:1125:19)
at TypeResolver.calcRefTypeName (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:685:39)
at TypeResolver.calcTypeReferenceTypeName (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:876:34)
at TypeResolver.getReferenceType (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:886:27)
at TypeResolver.resolve (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:513:36)
at TypeResolver.resolve (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/typeResolver.js:503:118)
at MethodGenerator.Generate (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/methodGenerator.js:62:78)
at /home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/controllerGenerator.js:46:41
at Array.map (<anonymous>)
at ControllerGenerator.buildMethods (/home/paima/paima-engine/node_modules/@tsoa/cli/dist/metadataGeneration/controllerGenerator.js:46:14)
npm ERR! Lifecycle script `compile:api` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @paima/[email protected]
npm ERR! at location: /home/paima/paima-engine/packages/engine/paima-rest
npm ERR! Lifecycle script `prebuild` failed with error:
npm ERR! Error: command failed
npm ERR! in workspace: @paima/[email protected]
npm ERR! at location: /home/paima/paima-engine/packages/engine/paima-rest
———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————
> NX Ran target prebuild for 2 projects (4s)
✔ 1/2 succeeded [0 read from cache]
✖ 1/2 targets failed, including the following:
- nx run @paima/rest:prebuild
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
display_name TEXT NOT NULL, | ||
description TEXT NOT NULL DEFAULT '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we want to include these two in the db as-is despite the fact they're language-specific depending on the header field you use when querying the achievement endpoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a table for language-agnostic info + another one for per-language info works for you, we can do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In an ideal world translations wouldn't be in the database at all, but if it has to be, probably it's best for it to be in a separate table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible alternatives:
- Another export from
packaged/endpoints.cjs
- Another
.json
file inpackaged/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I find putting "code" (stuff that's defined by the programmer and not by user input) in the database kind of goofy, this is what I'm looking at now:
- Game ID, achievement types, and languages are an additional
packaged/endpoints.cjs
export- But it's just a big data object, not a class with methods on it
- Achievement progress is stored in a DB table
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
packages/engine/paima-rest/src/controllers/AchievementsController.ts
Outdated
Show resolved
Hide resolved
* Tweak tsoa imports per todo comment * Add AchievementsController * Include time in getValidity * Log API errors to make debugging actually possible * Move AchievementService to utils-backend so it can be imported * Support games that export 'AchievementService' classes * Fix eslint errors * Interfaces too * Run pgtyped against a temporary Postgres instance * Add achievement tables and queries * Use new SQL queries in AchievementsController, remove AchievementService * Update for PRC-1 changes * Add table defs for new tables, clarify TODOs, fix issues * Update paima-db README to describe Docker * Use getMainAddress in /wallet/X * Add achievement_language table to back Accept-Language * TODO -> Future in AchievementsController * Improve import.ts documentation * Move checkedForPackedGameCode, rename importTsoa -> importEndpoints * Use import instead of DB for constant achievement definitions * Fix bugs and linter errors * Fix table definition mismatch * Add setAchievementProgress query * Use wallet ID instead of address to handle delegation
Approach:
paima-rest
defines a PRC-1 compliant/achievements
controllerachievements
export fromendpoints.cjs
achievement_progress
achievement_progress
as neededpaima-db
exports thesetAchievementProgress
query for this purposeBenefits:
utils-backend
for easy import by games in a pinchFor future improvement:
eip155
in the chain ID, which won't suffice when we add data availability layer support