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

Use plain strings for map zones #1042

Merged
merged 6 commits into from
Sep 28, 2024
Merged

Use plain strings for map zones #1042

merged 6 commits into from
Sep 28, 2024

Conversation

tsa96
Copy link
Member

@tsa96 tsa96 commented Sep 27, 2024

Refactors backend/DB so that zones are stored as plain text rather than jsonb.

With incoming map cache changes, we perform a hash check on map load to check zone files match the hash on the zoneHash field of the MapVersion table. Currently, zones are stored in Postgres as jsonb, a Postgres-specific binary serialization of JSON. It doesn't store whitespace, so makes storing zone hashes unreliable - it's probably possible to get the hash check to be consistent but I was struggling to get it working and felt very messy.

The main purpose of jsonb is to allow queries over that JSON data, which for zones we never need to do - we're really just using using the field for storage. The only thing we gain from using jsonb is that Prisma queries automatically parse the JSON into JSOs for us; this change requires we stick some JSON.parses in the backend code, but it's really not a big deal.

I haven't spent the time benching/profiling anything, but I suspect this method is quite a lot more performant overall, since the maps/<mapID>/zones GETs can just return raw strings, rather than having to serialize and deserialize each time, and that endpoint is called very frequently by the game.

Checks

  • !! DONT IGNORE ME !! I have ran nx run db:create-migration <name> and committed the migration if I've made DB schema changes
  • I have included/updated tests where applicable (see Testing)
  • I have followed semantic commit messages e.g. feat: Add foo, chore: Update bar, etc...
  • My branch has a clear history of changes that can be easy to follow when being reviewed commit-by-commit
  • My branch is functionally complete; the only changes to be done will be those potentially requested in code review
  • All changes requested in review have been fixuped into my original commits

@tsa96 tsa96 force-pushed the refactor/map-zone-strings branch 2 times, most recently from 6cc3cff to 02bbe35 Compare September 27, 2024 17:25
@tsa96 tsa96 marked this pull request as ready for review September 27, 2024 17:39
Can aaaaalmost get rid of lodash completely but need getDeep still,
don't have time to port atm
Trying to make CI fail if migrations aren't committed
My bad, accidently committed the uncommented version of this recently.
Probably best we just have an arg!
@tsa96 tsa96 merged commit 9cfb436 into main Sep 28, 2024
7 checks passed
@tsa96 tsa96 deleted the refactor/map-zone-strings branch September 28, 2024 13:13
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.

2 participants