-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat(store,world): combine schema and metadata registration, rename getSchema to getValueSchema, change Schema table id #1182
Conversation
🦋 Changeset detectedLatest commit: 3ce006c The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bytes32 constant _tableId = bytes32(abi.encodePacked(bytes16("mudstore"), bytes16("TableData"))); | ||
bytes32 constant TableDataTableId = _tableId; | ||
|
||
struct TableDataData { |
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.
not blocking but "data data" is a little weird
would calling this table Table
be too weird/confusing?
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.
we'd get TableTableId
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 about TableMetadata
or StoreMetadata
? (since the old table can go away, and the new one can get an extra field - tableName
)
packages/store/mud.config.ts
Outdated
abiEncodedKeyNames: "bytes", | ||
abiEncodedValueNames: "bytes", | ||
}, | ||
}, | ||
StoreMetadata: { |
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.
this can go away now, right?
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
f433947
to
233c35c
Compare
0b8a66d
to
7ff27a1
Compare
233c35c
to
d3a7d26
Compare
7ff27a1
to
4d918cd
Compare
c628440
to
f862270
Compare
4d918cd
to
4a52c4a
Compare
f862270
to
5908a3d
Compare
4a52c4a
to
c49c11e
Compare
5908a3d
to
20e5702
Compare
c49c11e
to
8a2d0d1
Compare
b84618f
to
840d930
Compare
8a2d0d1
to
0ca3474
Compare
0ca3474
to
2dc150b
Compare
9b1c72a
to
44fab13
Compare
world, | ||
{ table: Type.T }, | ||
{ metadata: { componentName: "TableMetadata" } } | ||
{ metadata: { componentName: "Tables" } } |
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.
I think this got renamed by mistake!
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.
Might be worth adjusting this name anyway to like RegisteredClientTables
or something
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.
That said, I wonder if the now-built-in Tables
component (derived from mud config) would have everything I need (minus some nice-to-haves like an already parsed table schema).
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.
good catch!
StoreMetadata.set(StoreCoreInternal.SCHEMA_TABLE, "schema", abi.encode(fieldNames)); | ||
// Register internal tables | ||
Tables.register(); | ||
Hooks.register(); |
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.
:chefkiss:
Co-authored-by: Kevin Ingersoll <kingersoll@gmail.com>
Co-authored-by: Kevin Ingersoll <kingersoll@gmail.com>
merging since only comment changes since last approval |
Fixes #1122
Fixes #1043
Fixes #1134
This PR includes a few breaking changes:
We need to integrate these changes into our networking stack before merging, so this PR is blocked on replacing v1 networking code with v2 (otherwise we'd have to integrate these chagnes into the deprecated v1 networking code which will be removed very soon).
Consumers who are using default MUD table libraries don't need to do anything to integrate these breaking changes except update to the latest version and regenerate the table libraries (which happens automatically during the build step of projects started from MUD templates)