feat(store,world): replace ResourceType table with ResourceId table and corresponding checks#1557
Conversation
🦋 Changeset detectedLatest commit: 3e1c724 The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 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 |
| string[] memory fieldNames | ||
| ) internal { | ||
| // Verify the table ID is of type RESOURCE_TABLE | ||
| if (!tableId.isType(RESOURCE_TABLE)) { |
There was a problem hiding this comment.
should this also include offchain tables?
There was a problem hiding this comment.
is it worth including a free function in storeResourceTypes.sol something like
function isTable(ResourceId resourceId) { ... }
that we can use here? or is the indirection too gassy?
There was a problem hiding this comment.
increases gas by 40, see ef89c43 - not super significant, but still don't think it's worth the small increase in ergonomics (agree it would read nicer, sad that the Solidity compiler doesn't just optimize this away)
4e4c8b5 to
522b070
Compare
cc53dbc to
c64ac48
Compare
…nd corresponding checks
6574e1f to
f89b5c8
Compare
| @@ -7,6 +7,7 @@ interface IStoreErrors { | |||
| // Errors include a stringified version of the tableId for easier debugging if cleartext tableIds are used | |||
| error StoreCore_TableAlreadyExists(ResourceId tableId, string tableIdString); | |||
| error StoreCore_TableNotFound(ResourceId tableId, string tableIdString); | |||
There was a problem hiding this comment.
nice, we already had this pattern for table errors (ID + human readable string)
Fixes #1553
TODO: