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

feat(store): rename StoreCore.registerCoreTables to registerInternalTables #2225

Merged
merged 6 commits into from Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chatty-timers-hug.md
@@ -0,0 +1,5 @@
---
"@latticexyz/world": patch
---

Updated `StoreCore` usage after rename and updated `CoreModule`'s internal methods for consistency with `StoreCore`.
5 changes: 5 additions & 0 deletions .changeset/twelve-terms-lay.md
@@ -0,0 +1,5 @@
---
"@latticexyz/store": major
---

Renamed `StoreCore`'s `registerCoreTables` method to `registerInternalTables`.
6 changes: 3 additions & 3 deletions docs/pages/store/reference/store-core.mdx
Expand Up @@ -21,15 +21,15 @@ StoreSwitch uses internal methods to write data instead of external calls._
function initialize() internal;
```

### registerCoreTables
### registerInternalTables

Register core tables in the store.
Register Store protocol's internal tables in the store.

_Consumers must call this function in their constructor before setting
any table data to allow indexers to decode table events._

```solidity
function registerCoreTables() internal;
function registerInternalTables() internal;
```

### getFieldLayout
Expand Down
4 changes: 2 additions & 2 deletions packages/store/src/StoreCore.sol
Expand Up @@ -88,11 +88,11 @@ library StoreCore {
}

/**
* @notice Register core tables in the store.
* @notice Register Store protocol's internal tables in the store.
* @dev Consumers must call this function in their constructor before setting
* any table data to allow indexers to decode table events.
*/
function registerCoreTables() internal {
function registerInternalTables() internal {
// Because `registerTable` writes to both `Tables` and `ResourceIds`, we can't use it
// directly here without creating a race condition, where we'd write to one or the other
// before they exist (depending on the order of registration).
Expand Down
2 changes: 1 addition & 1 deletion packages/store/test/StoreMock.sol
Expand Up @@ -18,7 +18,7 @@ import { ResourceId } from "../src/ResourceId.sol";
contract StoreMock is IStore, StoreData {
constructor() {
StoreCore.initialize();
StoreCore.registerCoreTables();
StoreCore.registerInternalTables();
StoreSwitch.setStoreAddress(address(this));
}

Expand Down
28 changes: 14 additions & 14 deletions packages/world/src/modules/core/CoreModule.sol
Expand Up @@ -62,8 +62,8 @@ contract CoreModule is Module {
* @dev Registers core tables, systems, and function selectors in the World.
*/
function installRoot(bytes memory) public override {
_registerCoreTables();
_registerCoreSystems();
_registerInternalTables();
_registerInternalSystems();
_registerFunctionSelectors();
}

Expand All @@ -76,11 +76,11 @@ contract CoreModule is Module {
}

/**
* @notice Register core tables in the World.
* @notice Register World's internal tables.
* @dev This internal function registers various tables and sets initial permissions.
*/
function _registerCoreTables() internal {
StoreCore.registerCoreTables();
function _registerInternalTables() internal {
StoreCore.registerInternalTables();
NamespaceOwner.register();
Balances.register();
InstalledModules.register();
Expand Down Expand Up @@ -108,20 +108,20 @@ contract CoreModule is Module {
}

/**
* @notice Register the core systems in the World.
* @notice Register the internal systems in the World.
*/
function _registerCoreSystems() internal {
_registerSystem(accessManagementSystem, ACCESS_MANAGEMENT_SYSTEM_ID);
_registerSystem(balanceTransferSystem, BALANCE_TRANSFER_SYSTEM_ID);
_registerSystem(batchCallSystem, BATCH_CALL_SYSTEM_ID);
_registerSystem(coreRegistrationSystem, CORE_REGISTRATION_SYSTEM_ID);
function _registerInternalSystems() internal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels a little bit off to call these "internal systems" - with that name I'd think of internal functions, ie something that's not externally accessible, but these systems become part of the "core" API 🙈

Copy link
Member Author

@holic holic Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're in the CoreModule (soon InitModule), maybe we can just name these _registerTables, _registerSystems, and _registerSystem because that's what installing the module does? _registerFunctionSelectors is already named as such.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

went ahead and did this in e58b76f

_registerInternalSystem(accessManagementSystem, ACCESS_MANAGEMENT_SYSTEM_ID);
_registerInternalSystem(balanceTransferSystem, BALANCE_TRANSFER_SYSTEM_ID);
_registerInternalSystem(batchCallSystem, BATCH_CALL_SYSTEM_ID);
_registerInternalSystem(coreRegistrationSystem, CORE_REGISTRATION_SYSTEM_ID);
}

/**
* @notice Register the core system in the World.
* @dev Uses the CoreRegistrationSystem's `registerSystem` implementation to register the system on the World.
* @notice Register the internal system in the World.
* @dev Uses the WorldRegistrationSystem's `registerSystem` implementation to register the system on the World.
*/
function _registerSystem(address target, ResourceId systemId) internal {
function _registerInternalSystem(address target, ResourceId systemId) internal {
WorldContextProviderLib.delegatecallWithContextOrRevert({
msgSender: _msgSender(),
msgValue: 0,
Expand Down