db: ordered migrations, user_items table, and shared inventory helpers - #32
Open
Seltraeh wants to merge 1 commit into
Open
db: ordered migrations, user_items table, and shared inventory helpers#32Seltraeh wants to merge 1 commit into
Seltraeh wants to merge 1 commit into
Conversation
Migrations previously lived in an unordered_map and therefore ran in hash order. They are now a vector so they run in declaration order, with a startup guard that aborts on a duplicate migration name to preserve the uniqueness the map gave for free. Adds 02072026_ExtendUserUnitsForUnitOps: 26 additive columns on user_units covering per-unit stats, the two sphere equipment slots and the favourite flag. This consolidates three earlier fork migrations onto the upstream table shape. Upstream columns (unit_lvl, base_rec, bb_*) remain the source of truth for upstream handlers; these serve handlers not yet moved to PacketInterface and are intended to be consolidated away as each one moves. Adds 05072026_CreateUserItemsTable: one row per item stack, keyed UNIQUE(user_id, item_id), with instance_id as the warehouse row id the client references. Common.hpp gains two helpers. addUserItem() upserts a stack, incrementing item_num when the species is already owned. returnEquippedSpheres() reads the sphere slots off units that are about to be deleted and credits them back to the warehouse; it must be called before the DELETE in any handler that consumes units, or the spheres are destroyed silently.
Tom2096
reviewed
Aug 6, 2026
| // shape. Upstream columns (unit_lvl/base_rec/ext_rec/bb_*) remain the | ||
| // source of truth for upstream handlers; these serve the not-yet-ported | ||
| // quests handlers and are consolidated away as each moves to | ||
| // PacketInterface. |
Collaborator
There was a problem hiding this comment.
I dont understand why this exists, a lot of it seems like duplicates of user units.
Tom2096
reviewed
Aug 6, 2026
| .insert = true, | ||
| }), | ||
| field<&::UserUnitInfo::ext_rec>("ext_rec", { | ||
| field<&::UserUnitInfo::ext_rec>("ext_heal", { |
Tom2096
reviewed
Aug 6, 2026
| .update = true, | ||
| .insert = true, | ||
| }), | ||
| // Extras the read/display path was missing — mapped to the quests mirror |
Collaborator
There was a problem hiding this comment.
i actually dont agree with adding everything here - it introduces bloat
for example, do we really need to add fe_bp? or leader_skill_id?
for each of these fields, we should only add them under 2 conditiions (this applies to every single other field as well)
- we understand fully what it does in the client
- the client needs it either for game functionailty or it crashes
Tom2096
reviewed
Aug 6, 2026
|
|
||
| // UPSERT: bump the stack if it exists, else insert a new one. item_id and | ||
| // quantity are server-trusted integers, safe to bind. | ||
| auto result = co_await database->execSqlCoro( |
Collaborator
There was a problem hiding this comment.
dont use co_await database->execSqlCoro(, use the existing API
Tom2096
reviewed
Aug 6, 2026
| if (!database || identity.userId.empty() || userUnitIdList.empty()) | ||
| co_return; | ||
|
|
||
| const auto rows = co_await database->execSqlCoro( |
Collaborator
There was a problem hiding this comment.
again dont use co_await database->execSqlCoro(
Tom2096
reviewed
Aug 6, 2026
| } | ||
|
|
||
| /*! | ||
| * Returns any spheres equipped on soon-to-be-consumed units to the owner's |
Collaborator
There was a problem hiding this comment.
I dont understand the mechanism here - does the client delete the sphere if we fuse the unit or sell the unit?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
db: ordered migrations, user_items table, and shared inventory helpers
Branch:
split/03-schema-foundationBase:
devMerge position: 03 of 13
Three files, but the highest-risk change in the whole series. Everything downstream depends on these table shapes.
What's included
Migration ordering. Migrations lived in an
unordered_mapand therefore ran in hash order. They are now avector, running in declaration order. Because a vector does not dedup, a startup guard aborts on a duplicate migration name, preserving the uniqueness the map gave for free.02072026_ExtendUserUnitsForUnitOps— 26 additive columns onuser_units: per-unit stats, two sphere equipment slots, and the favourite flag. Consolidates three earlier fork migrations onto the upstream table shape.05072026_CreateUserItemsTable— one row per item stack,UNIQUE(user_id, item_id), withinstance_idas the warehouse row id the client references.Structural tables for the later PRs — town, campaign, scenario and summon-ticket tables land here too, so
MigrationManager.cppis touched once rather than by five branches. All areCREATE TABLE IF NOT EXISTS, structural only, never seeded. Say the word if you'd rather each subsystem carried its own and I'll redistribute.Common.hpphelpersaddUserItem()— upserts a stack, incrementingitem_numwhen the species is already owned.returnEquippedSpheres()— reads sphere slots off units about to be deleted and credits them back to the warehouse. Must be called before the DELETE in any handler that consumes units.Known debt, flagging deliberately
The 26
user_unitscolumns duplicate upstream'sunit_lvl/base_rec/bb_*. Upstream columns remain the source of truth for upstream handlers; these serve handlers not yet onPacketInterface. The intent is to consolidate them away as each handler moves. If you'd rather that port happened before any of this merges, that's a reasonable call and I'd like to know now rather than later.Verification
Fresh-DB boot runs every migration in declared order; second boot skips recorded ones. Duplicate-name guard tested by temporarily doubling an entry.