-
Notifications
You must be signed in to change notification settings - Fork 196
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(cli): add trace #999
feat(cli): add trace #999
Conversation
yeah if it's not needed for this PR, it'd be great to remove this or separate it |
packages/cli/src/commands/trace.ts
Outdated
|
||
const labels: { name: string; address: string }[] = []; | ||
for (const name of names) { | ||
const tableId = new TableId(namespace, name); |
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.
unrelated to this PR but we should probably rename this util from TableId
to ResourceSelector
Co-authored-by: alvarius <[email protected]>
packages/cli/src/commands/trace.ts
Outdated
import { existsSync, readFileSync } from "fs"; | ||
import type { CommandModule } from "yargs"; | ||
import { ethers } from "ethers"; | ||
|
||
import { loadConfig } from "@latticexyz/config/node"; | ||
import { MUDError } from "@latticexyz/common/errors"; | ||
import { cast, getRpcUrl, getSrcDirectory } from "@latticexyz/common/foundry"; | ||
import { TableId } from "@latticexyz/utils"; | ||
import { StoreConfig } from "@latticexyz/store"; | ||
import { resolveWorldConfig, WorldConfig } from "@latticexyz/world"; | ||
import { IBaseWorld } from "@latticexyz/world/types/ethers-contracts/IBaseWorld"; | ||
import { getChainId, getExistingContracts } from "../utils"; | ||
|
||
import IBaseWorldData from "@latticexyz/world/abi/IBaseWorld.sol/IBaseWorld.json" assert { type: "json" }; | ||
|
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.
spacing was supposed to mirror deploy.ts
, but then I added some deps and forgot em. wdyt now?
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'm not sure why spacing is like this in deploy.ts
either haha, if we put spacing between imports i'd expect to have a group for external dependencies, maybe a separate one for mud dependencies, and maybe a third one for local imports. So in this case i'd expect the IBaseWorldData
import to be in the same group as the other mud imports, and then maybe a space between that group and ../utils
, but no strong opinion about that.
packages/cli/src/commands/trace.ts
Outdated
const tableId = new TableId(namespace, name); | ||
// Get the first field of `Systems` table (the table maps system name to its address and other data) | ||
const address = await WorldContract.getField(systemsTableId.toHexString(), [tableId.toHexString()], 0); |
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.
The TableId
class should probably be called ResourceSelector
, but even without that change we shouldn't call the variable tableId
here to avoid confusion
const tableId = new TableId(namespace, name); | |
// Get the first field of `Systems` table (the table maps system name to its address and other data) | |
const address = await WorldContract.getField(systemsTableId.toHexString(), [tableId.toHexString()], 0); | |
const systemSelector = new TableId(namespace, name); | |
// Get the first field of `Systems` table (the table maps system name to its address and other data) | |
const address = await WorldContract.getField(systemsTableId.toHexString(), [systemSelector.toHexString()], 0); |
|
fixes #713
also updates pnpm (this wasn't very intentional, do u want it in a separate PR? )(moved this to #1000)and refactors existingContracts a bit, since it was gathering a lot of code repetition with trace also using it