-
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: rename table to tableId #1484
Conversation
🦋 Changeset detectedLatest commit: 8854d92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 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 |
58ef87c
to
6edf524
Compare
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
* Note: if a tableId with composite keys is used, only the first key is indexed | ||
*/ | ||
contract KeysInTableHook is StoreHook { | ||
function handleSet(bytes32 tableId, bytes32[] memory key) internal { | ||
bytes32 keysHash = keccak256(abi.encode(key)); | ||
|
||
// If the key has not yet been set in the table... | ||
// If the key has not yet been set in the tableId... | ||
if (!UsedKeysIndex.getHas(tableId, keysHash)) { | ||
uint40 length = uint40(KeysInTable.lengthKeys0(tableId)); | ||
|
||
// Push the key to the list of keys in this table | ||
// Push the key to the list of keys in this tableId |
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 a bunch of comments here (more below) should retain table
} | ||
|
||
function setUp() public { | ||
// Register table's value schema | ||
// Register tableId's value 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.
this one too
I may have missed some, didn't go through every comment |
9348d50
to
26df50f
Compare
eb6ff44
to
bb8cc29
Compare
bb8cc29
to
84716aa
Compare
4189c86
to
14582af
Compare
48c5fdc
to
09755c6
Compare
bytes32 table2 = keccak256("other.table"); | ||
StoreCore.registerTable(table, fieldLayout, defaultKeySchema, valueSchema, new string[](1), new string[](4)); | ||
bytes32 tableId = keccak256("some.tableId"); | ||
bytes32 table2 = keccak256("other.tableId"); |
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.
bytes32 table2 = keccak256("other.tableId"); | |
bytes32 tableId2 = keccak256("other.tableId"); |
|
||
startGasReport("Check for existence of table (existent)"); | ||
StoreCore.hasTable(table); | ||
startGasReport("Check for existence of tableId (existent)"); |
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.
startGasReport("Check for existence of tableId (existent)"); | |
startGasReport("Check for existence of table (existent)"); |
endGasReport(); | ||
|
||
startGasReport("check for existence of table (non-existent)"); | ||
startGasReport("check for existence of tableId (non-existent)"); |
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.
startGasReport("check for existence of tableId (non-existent)"); | |
startGasReport("check for existence of table (non-existent)"); |
endGasReport(); | ||
} | ||
|
||
function testSetAndGetStaticDataSpanningWords() public { | ||
// Register table | ||
// Register tableId |
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.
// Register tableId | |
// Register table |
|
||
// Register table | ||
// Register tableId |
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.
// Register tableId | |
// Register table |
|
||
// Register table | ||
// Register tableId |
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.
// Register tableId | |
// Register table |
(there's a bunch more of this shape I didn't comment on)
@@ -634,34 +634,34 @@ contract StoreCoreGasTest is Test, GasReporter, StoreMock { | |||
|
|||
bytes memory data = abi.encodePacked(bytes16(0x0102030405060708090a0b0c0d0e0f10)); | |||
|
|||
startGasReport("set record on table with subscriber"); | |||
StoreCore.setRecord(table, key, data, fieldLayout); | |||
startGasReport("set record on tableId with subscriber"); |
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.
startGasReport("set record on tableId with subscriber"); | |
startGasReport("set record on table with subscriber"); |
endGasReport(); | ||
|
||
data = abi.encodePacked(bytes16(0x1112131415161718191a1b1c1d1e1f20)); | ||
|
||
startGasReport("set static field on table with subscriber"); | ||
StoreCore.setField(table, key, 0, data, fieldLayout); | ||
startGasReport("set static field on tableId with subscriber"); |
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.
startGasReport("set static field on tableId with subscriber"); | |
startGasReport("set static field on table with subscriber"); |
(there's a few more of this shape I didn't comment on)
@@ -8,7 +8,7 @@ import { KeysInTable } from "./tables/KeysInTable.sol"; | |||
import { UsedKeysIndex } from "./tables/UsedKeysIndex.sol"; | |||
|
|||
/** | |||
* Note: if a table with composite keys is used, only the first key is indexed | |||
* Note: if a tableId with composite keys is used, only the first key is indexed |
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.
* Note: if a tableId with composite keys is used, only the first key is indexed | |
* Note: if a table with composite keys is used, only the first key is indexed |
The base branch was changed.
table
totableId
wherever it is used as an IDhttps://www.notion.so/latticexyz/MUD-naming-conventions-66e11356a1934465b2aa1a951c61a299