-
Notifications
You must be signed in to change notification settings - Fork 45
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: add some documentation details #290
Conversation
WalkthroughThis pull request introduces new TypeScript interfaces and an enumeration to the Dojo SDK, enhancing type safety and structure. It includes the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
packages/sdk/readme.md (5)
90-125
: LGTM! Consider a minor naming consistency improvement.The newly added
Direction
enum and interfaces (Moves
,DirectionAvailable
,Pos
,Position
, andMockSchemaType
) are well-defined and consistent with the expected schema structure. They provide a clear type definition for the Dojo SDK.For consistency, consider renaming the
DirectionAvailable
interface toDirectionsAvailable
to match the usage in the schema object below (line 140). This will ensure perfect alignment between the interface and the schema definition.
Line range hint
128-150
: UpdateMoves
model for consistency with its interface.The schema object is correctly updated to include the new models under the
dojo_starter
namespace. However, there's a minor inconsistency in theMoves
model:The
Moves
model in the schema includes aremaining
field (line 134) that is not present in theMoves
interface (lines 98-103). To maintain consistency, either:
- Add the
remaining
field to theMoves
interface:interface Moves { fieldOrder: string[]; player: string; remaining: number; // Add this line last_direction: Direction; can_move: boolean; }
- Or remove the
remaining
field from the schema object:Moves: { fieldOrder: ["player", "last_direction", "can_move"], player: "", last_direction: Direction.None, can_move: false, },Please choose the option that best reflects the intended structure of the
Moves
model.
Line range hint
152-180
: LGTM! Consider adding a note about type generation.The updated SDK initialization example correctly demonstrates how to use the new
MockSchemaType
interface. This provides a clear guide for developers on how to initialize the SDK with the new schema structure.To further improve clarity, consider adding a note reminding developers to regenerate their TypeScript types using
sozo build --typescript-v2
after making changes to their world schema. This will ensure that theMockSchemaType
stays in sync with the actual schema. You could add this note just before the initialization example:**Note:** Remember to regenerate your TypeScript types using `sozo build --typescript-v2` after making changes to your world schema to keep your `MockSchemaType` up to date.
190-197
: LGTM! Consider adjusting the markdown formatting.The added table of query operators provides a clear and concise overview of the available operators for filtering in queries. This is valuable information for developers using the SDK.
To address the Markdownlint hint and improve consistency with the rest of the document, consider changing the indented code block to a fenced code block. Here's how you can format it:
| Operator | Description | | -------- | ------------------------ | | `$eq` | Equal to | | `$neq` | Not equal to | | `$gt` | Greater than | | `$gte` | Greater than or equal to | | `$lt` | Less than | | `$lte` | Less than or equal to |This change will resolve the Markdownlint warning and maintain consistent formatting throughout the document.
🧰 Tools
🪛 Markdownlint
190-190: Expected: fenced; Actual: indented
Code block style(MD046, code-block-style)
269-272
: LGTM! Consider adding an example configuration.The added note about the
relayUrl
is crucial for developers to understand how to properly configure message broadcasting. This information aligns well with the SDK's functionality and helps prevent potential issues.To further enhance clarity, consider adding a brief example of how to set the
relayUrl
in theinit
function. This could be added right after the existing note:// Example of setting the relayUrl in the init function const db = await init<MockSchemaType>( { client: { // ... other configuration options relayUrl: "/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Fonchain-dash%2Ftorii%2Fwss", }, // ... rest of the configuration }, schema );This example will provide developers with a clear reference on how to properly set the
relayUrl
in their configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/sdk/readme.md (5 hunks)
🧰 Additional context used
🪛 Markdownlint
packages/sdk/readme.md
190-190: Expected: fenced; Actual: indented
Code block style(MD046, code-block-style)
Summary by CodeRabbit
New Features
Direction
with values:None
,Up
,Down
,Left
, andRight
.Moves
,DirectionAvailable
,Pos
,Position
, andMockSchemaType
for better game state management.Documentation
relayUrl
in theinit
function for sending signed messages.