Skip to content

Conversation

bengineer42
Copy link
Contributor

@bengineer42 bengineer42 commented Jul 8, 2025

Description

Added the write schema method to dojo to write part of a model efficiently

Related issue

Tests

  • Yes
  • No, because they aren't needed
  • No, because I need help

Added to documentation?

  • README.md
  • Dojo Book
  • No documentation needed

Checklist

  • I've formatted my code (scripts/rust_fmt.sh, scripts/cairo_fmt.sh)
  • I've linted my code (scripts/clippy.sh, scripts/docs.sh)
  • I've commented my code
  • I've requested a review after addressing the comments

Summary by CodeRabbit

  • New Features

    • Added support for schema-based partial model updates and batch member updates, including new methods for writing and reading schemas.
    • Introduced a new event type for tracking batch member updates.
    • Added new schema and model structures for more flexible data handling.
    • Added a new method to update player configuration items in the example project.
    • Added new configuration file for example project setup.
  • Bug Fixes

    • Improved event handling and serialization for new schema events.
  • Tests

    • Added comprehensive tests for schema-based reading and writing, including equality checks and partial updates.
  • Documentation

    • Updated manifests and configuration files to reflect new features and changes.
  • Style

    • Enhanced code style with additional trait derivations for equality comparisons.
    • Reformatted serialization and deserialization code for improved readability.

- Updated dojo and dojo_plugin versions in Scarb.lock and Scarb.toml to 1.5.1 and 2.10.1 respectively.
- Modified model.cairo to include new LargeQuintupleSchema and related constants for enhanced data handling.
- Added new tests for reading and writing quintuple models, values, and schemas in model.cairo.
- Enhanced actions.cairo with a new function to update player configuration items, allowing for better management of player inventory.
- Introduced PlayerConfigItems struct in models.cairo to encapsulate player items and their favorite item.
Copy link
Contributor

coderabbitai bot commented Jul 8, 2025

Walkthrough

Ohayo sensei! This update introduces schema-based batch operations for models, new event handling for StoreUpdateMembers, and adds support for partial model writes and reads using schemas. It also extends public traits and enums, updates contract logic, and enhances test coverage for schema operations and equality checks.

Changes

Files/Group Change Summary
bin/sozo/src/commands/events.rs Added handling and formatting for WorldEvent::StoreUpdateMembers in match_event.
crates/dojo/core/src/lib.cairo, crates/dojo/core/src/meta/layout.cairo Changed public exports; added/renamed traits and methods for field layouts and schema support.
crates/dojo/core/src/model/definition.cairo Added Schema variant to ModelIndex enum.
crates/dojo/core/src/model/model.cairo Added to_schemas method to ModelPtrsTrait and its implementation.
crates/dojo/core/src/model/storage.cairo, crates/dojo/core/src/world/storage.cairo Added write_schema/write_schemas methods; updated schema-based read/write logic.
crates/dojo/core/src/world/errors.cairo Added DELETE_ENTITY_SCHEMA error constant.
crates/dojo/core/src/world/world_contract.cairo Added StoreUpdateMembers event struct/variant; extended contract logic for schema batch updates and events.
crates/dojo/world/src/contracts/abigen/world.rs Added StoreUpdateMembers struct/variant; updated serialization, deserialization, and event matching logic.
examples/benchmark/src/model.cairo, crates/dojo/core-tests/src/tests/model/model.cairo Added PartialEq derives, new schema structs, constants, and comprehensive tests for schema operations.
examples/spawn-and-move/manifest_dev.json Updated manifest: new addresses, Schema variant, StoreUpdateMembers event, new contract function.
examples/spawn-and-move/src/actions.cairo Added update_player_config_items method to actions interface and implementation.
examples/spawn-and-move/src/models.cairo Added PlayerConfigItems struct.
examples/spawn-and-move/torii_dev.toml Added new configuration file for Torii indexer.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Contract
    participant Storage

    User->>Contract: set_entity_internal(ModelIndex::Schema, schema_data)
    Contract->>Storage: write_struct_layout(schema_data)
    Contract->>Contract: emit StoreUpdateMembers event (selector, entity_id, member_selectors, values)
Loading
sequenceDiagram
    participant User
    participant Contract

    User->>Contract: delete_entity_internal(ModelIndex::Schema)
    Contract->>Contract: panic(DELETE_ENTITY_SCHEMA)
Loading

Possibly related PRs

Suggested labels

contributor

Suggested reviewers

  • glihm
  • kariy

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca45360 and 2eb84d1.

📒 Files selected for processing (3)
  • crates/dojo/world/src/contracts/abigen/model.rs (16 hunks)
  • crates/dojo/world/src/contracts/abigen/world.rs (106 hunks)
  • examples/spawn-and-move/src/actions.cairo (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • examples/spawn-and-move/src/actions.cairo
  • crates/dojo/world/src/contracts/abigen/world.rs
🔇 Additional comments (7)
crates/dojo/world/src/contracts/abigen/model.rs (7)

82-87: Ohayo sensei! LGTM - Clean formatting improvement

The multi-line formatting with trailing commas improves readability in the auto-generated cairo_serialize method.


102-106: Consistent struct construction formatting looks good

The multi-line struct construction with trailing commas maintains consistency with the serialization formatting above.


126-128: Excellent consistent formatting across all serialize methods

All the cairo_serialize method calls have been consistently formatted to use multi-line style with trailing commas. This greatly improves readability and maintainability of the auto-generated code.

Also applies to: 164-166, 207-209, 259-267, 305-310, 348-350


231-237: Well-formatted struct construction in deserialize methods

The struct construction formatting is now consistent with the serialize methods, using multi-line style with trailing commas for better readability.

Also applies to: 281-285, 322-325, 365-369


411-414: Improved error message formatting in TryFrom implementations

The multi-line formatting of the error messages makes them more readable while maintaining the same functionality.

Also applies to: 424-427


505-529: Clean multi-line formatting in Layout enum deserialize method

The consistent multi-line formatting for the cairo_deserialize calls improves code readability while maintaining the same deserialization logic.


614-633: Consistent formatting applied to Ty enum deserialize method

The multi-line formatting matches the pattern used throughout the file, providing better readability for the auto-generated deserialization code.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
crates/dojo/world/src/contracts/abigen/world.rs (1)

1-5038: Ohayo sensei! Skipping review of auto-generated code.

This file is auto-generated by cainome and should not be manually edited. Any concerns should be addressed in the code generator itself.

🧹 Nitpick comments (3)
bin/sozo/src/commands/events.rs (1)

372-392: Ohayo sensei! Consider improving the event label for clarity.

The implementation correctly handles the new StoreUpdateMembers event, but the label "Store update member" might be confusing since this variant handles multiple members. Consider changing it to "Store update members" to distinguish it from the singular StoreUpdateMember event.

                format!("Store update member ({})", tag),
+               format!("Store update members ({})", tag),
examples/spawn-and-move/manifest_dev.json (2)

1168-1194: Event layout: consider promoting member_selectors to a key for efficient filtering

StoreUpdateMembers only uses selector and entity_id as keys. If off-chain indexers will ever want to query by an individual member selector, emit it as a key rather than data. Confirm the UX before this gets frozen on-chain.


1514-1520: update_player_config_items ABI entry added – remember to expose it in docs/CLI

The wiring is fine; just ensure the CLI helpers and README pick up the new system.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1616678 and ba1ea2d.

⛔ Files ignored due to path filters (1)
  • examples/benchmark/Scarb.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • bin/sozo/src/commands/events.rs (1 hunks)
  • crates/dojo/core-cairo-test/src/tests/model/model.cairo (4 hunks)
  • crates/dojo/core/src/lib.cairo (1 hunks)
  • crates/dojo/core/src/meta/layout.cairo (2 hunks)
  • crates/dojo/core/src/model/definition.cairo (2 hunks)
  • crates/dojo/core/src/model/model.cairo (2 hunks)
  • crates/dojo/core/src/model/storage.cairo (1 hunks)
  • crates/dojo/core/src/world/errors.cairo (1 hunks)
  • crates/dojo/core/src/world/storage.cairo (2 hunks)
  • crates/dojo/core/src/world/world_contract.cairo (6 hunks)
  • crates/dojo/world/src/contracts/abigen/model.rs (16 hunks)
  • crates/dojo/world/src/contracts/abigen/world.rs (99 hunks)
  • examples/benchmark/Scarb.toml (2 hunks)
  • examples/benchmark/src/model.cairo (6 hunks)
  • examples/spawn-and-move/dojo_dev.toml (0 hunks)
  • examples/spawn-and-move/manifest_dev.json (10 hunks)
  • examples/spawn-and-move/src/actions.cairo (4 hunks)
  • examples/spawn-and-move/src/models.cairo (1 hunks)
  • examples/spawn-and-move/torii_dev.toml (1 hunks)
💤 Files with no reviewable changes (1)
  • examples/spawn-and-move/dojo_dev.toml
🧰 Additional context used
🧠 Learnings (14)
examples/benchmark/src/model.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
examples/spawn-and-move/src/actions.cairo (2)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in `crates/dojo/world/src/contracts/abigen/` is auto-generated and should be excluded from code reviews.
crates/dojo/world/src/contracts/abigen/model.rs (2)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in `crates/dojo/world/src/contracts/abigen/` is auto-generated and should be excluded from code reviews.
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
examples/spawn-and-move/manifest_dev.json (1)

undefined

<retrieved_learning>
Learnt from: glihm
PR: #2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in crates/dojo/world/src/contracts/abigen/ is auto-generated and should be excluded from code reviews.
</retrieved_learning>

crates/dojo/core/src/model/storage.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
examples/benchmark/Scarb.toml (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core-cairo-test/src/tests/model/model.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/meta/layout.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/lib.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/world/storage.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/model/definition.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/world/errors.cairo (1)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
crates/dojo/core/src/world/world_contract.cairo (2)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in `crates/dojo/world/src/contracts/abigen/` is auto-generated and should be excluded from code reviews.
crates/dojo/world/src/contracts/abigen/world.rs (1)

undefined

<retrieved_learning>
Learnt from: glihm
PR: #2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in crates/dojo/world/src/contracts/abigen/ is auto-generated and should be excluded from code reviews.
</retrieved_learning>

🧬 Code Graph Analysis (1)
crates/dojo/world/src/contracts/abigen/world.rs (5)
crates/dojo/world/src/contracts/abigen/model.rs (30)
  • cairo_serialize (79-89)
  • cairo_serialize (124-131)
  • cairo_serialize (161-169)
  • cairo_serialize (205-215)
  • cairo_serialize (257-269)
  • cairo_serialize (303-312)
  • cairo_serialize (345-353)
  • cairo_serialize (383-387)
  • cairo_serialize (454-489)
  • cairo_serialize (551-586)
  • cairo_serialized_size (72-78)
  • cairo_serialized_size (118-123)
  • cairo_serialized_size (154-160)
  • cairo_serialized_size (196-204)
  • cairo_serialized_size (250-256)
  • cairo_serialized_size (297-302)
  • cairo_serialized_size (338-344)
  • cairo_serialized_size (378-382)
  • cairo_serialized_size (443-453)
  • cairo_serialized_size (540-550)
  • cairo_deserialize (90-107)
  • cairo_deserialize (132-142)
  • cairo_deserialize (170-182)
  • cairo_deserialize (216-238)
  • cairo_deserialize (270-286)
  • cairo_deserialize (313-326)
  • cairo_deserialize (354-370)
  • cairo_deserialize (388-402)
  • cairo_deserialize (490-525)
  • cairo_deserialize (587-618)
crates/dojo/world/src/local/resource.rs (3)
  • name (87-95)
  • namespace (98-106)
  • class_hash (109-117)
crates/dojo/world/src/remote/resource.rs (4)
  • name (162-170)
  • namespace (173-181)
  • address (189-197)
  • metadata_hash (211-219)
crates/dojo/world/src/diff/resource.rs (3)
  • name (63-69)
  • namespace (72-78)
  • metadata_hash (117-123)
crates/dojo/types/src/schema.rs (2)
  • keys (584-586)
  • option (611-623)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: fmt
🔇 Additional comments (36)
examples/benchmark/src/model.cairo (4)

5-5: Ohayo sensei! Good addition of PartialEq for model comparisons.

Adding PartialEq to the model structs enables equality comparisons which are essential for the new test assertions. This is a necessary change for the comprehensive test coverage.

Also applies to: 13-13


44-52: LGTM! The new schema struct follows the established pattern.

The LargeQuintupleSchema struct correctly follows the same pattern as other schema structs, with appropriate derives and field selections (v0, v1, v3, v4, v5).


63-85: Well-structured test data constants, sensei!

The constants provide excellent coverage for different partial update scenarios:

  • SINGLE: Complete simple model
  • LARGE: Complete complex model
  • LARGE_SINGLE/DOUBLE/QUINTUPLE: Partial field sets matching their schema variants

This systematic approach to test data will ensure thorough validation of the schema-based write functionality.


102-566: Comprehensive test suite for schema-based operations!

Ohayo! The test suite provides excellent coverage across all dimensions:

  • Operations: read/write for models, values, schemas, and individual members
  • Model types: simple (Single) and complex (Large) models
  • Partial updates: single, double, quintuple, and sextuple field sets
  • Verification: Each write test validates the result with equality assertions

The systematic approach ensures the new schema write functionality works correctly for all use cases.

crates/dojo/core/src/world/errors.cairo (1)

4-4: Appropriate error constant for schema entity protection.

The new DELETE_ENTITY_SCHEMA error constant correctly prevents deletion of schema entities, which is essential since schemas define the structure of models and should not be deleted through entity operations.

crates/dojo/core/src/model/definition.cairo (2)

1-2: Clean import organization, sensei!

Splitting the imports into separate lines improves readability and follows best practices.


16-16: Essential addition of Schema variant to ModelIndex.

Ohayo! The new Schema: felt252 variant enables schema-based model indexing, which is fundamental to the PR's goal of supporting partial model updates through schemas. This addition integrates well with the existing index types.

examples/spawn-and-move/src/models.cairo (1)

80-84: Well-designed schema struct for partial PlayerConfig updates!

The PlayerConfigItems struct correctly extracts a subset of PlayerConfig fields (items and favorite_item), enabling partial updates through the new schema write functionality. The derives are appropriate for schema serialization and introspection.

examples/benchmark/Scarb.toml (1)

2-2: Consistent Cairo version updates across dependencies.

Ohayo sensei! The version bumps from 2.9.2 to 2.10.1 are consistently applied to all Cairo-related dependencies (cairo-version, starknet, cairo_test). This ensures compatibility with the new schema features while maintaining backward compatibility through a minor version increment.

Also applies to: 14-14, 17-17

examples/spawn-and-move/torii_dev.toml (1)

1-9: Ohayo sensei! LGTM - Clean development configuration setup.

The Torii development configuration looks solid with clear documentation. The world address format, localhost RPC endpoint, and optional database directory with fallback comment are all appropriate for a development environment.

crates/dojo/core/src/model/model.cairo (2)

25-25: Ohayo sensei! Great trait extension for schema support.

Adding the to_schemas method to the trait provides a clean API for schema-based model operations.


38-44: Ohayo sensei! LGTM - Clean implementation following established patterns.

The to_schemas implementation correctly mirrors the existing to_indexes method while using the ModelIndex::Schema variant. This provides the foundation for schema-based partial model updates.

crates/dojo/world/src/contracts/abigen/model.rs (1)

1-857: Ohayo sensei! Skipping review of auto-generated code.

This file is auto-generated as indicated by the header comment. Auto-generated code in the abigen/ directory should be excluded from code reviews as per project conventions.

crates/dojo/core/src/lib.cairo (1)

39-39: Ohayo sensei! LGTM - Clean trait export reorganization.

The updated exports properly expose the new FieldLayoutsTrait and LayoutTrait while removing the obsolete LayoutCompareTrait. This aligns well with the schema-based enhancements throughout the codebase.

crates/dojo/core-cairo-test/src/tests/model/model.cairo (2)

39-39: Ohayo sensei! LGTM on the PartialEq trait additions!

Adding PartialEq to these structs enables proper equality comparisons in the test assertions, which is essential for validating the schema write functionality.

Also applies to: 48-48, 58-58


309-322: Excellent test coverage for the new write_schema functionality!

This test perfectly demonstrates the core use case of partial model updates. The logic is sound:

  1. Write initial model
  2. Create schema with updated fields
  3. Use write_schema to update only specific members
  4. Verify the changes took effect

The test validates that schema writes correctly update underlying models without affecting other fields.

crates/dojo/core/src/model/storage.cairo (1)

64-70: Ohayo sensei! Solid trait design for schema write operations!

The method signatures are well-designed:

  • Proper generic constraints (Serde + Introspect)
  • Consistent with existing patterns (single + batch variants)
  • Clear documentation explaining the purpose
  • Symmetrical with the existing read_schema methods

This provides a clean API for partial model updates.

examples/spawn-and-move/src/actions.cairo (4)

8-8: Ohayo sensei! Great addition to the interface!

The new method signature properly extends the actions interface to support schema-based updates.


20-39: Nice import organization improvements!

The conditional compilation directives are properly grouped and the imports are well-organized by module. This improves readability and maintainability.


140-152: Excellent demonstration of the write_schema functionality!

This method showcases the key benefits of schema writes:

  • Direct field updates without reading the full model first
  • More efficient than read-modify-write pattern
  • Clear separation of concerns with the schema structure

The comment explaining the optimization is particularly helpful for other developers.


258-267: LGTM on the test import reorganization!

Grouping the imports logically improves code readability and follows good organization practices.

crates/dojo/core/src/meta/layout.cairo (3)

26-26: Ohayo sensei! Good trait renaming!

Renaming from LayoutCompareTrait to LayoutTrait makes sense since the trait now handles more than just comparisons. The more generic name better reflects its expanded purpose.


38-44: Well-implemented struct_fields method!

The method correctly extracts fields from Struct layouts with appropriate error handling. Panicking on wrong layout type is the right approach since this indicates a programming error.


46-55: Excellent utility trait for field selector extraction!

The FieldLayoutsTrait with its selectors method provides a clean way to extract field selectors from layout spans. The implementation is straightforward and efficient, using a simple loop to collect selectors into an array.

examples/spawn-and-move/manifest_dev.json (2)

140-146: Ohayo sensei – double-check every match on ModelIndex now that Schema exists

Adding the Schema variant is legit, but any match or from_repr() logic that enumerates ModelIndex will panic if it doesn’t include this arm. Grep the codebase for match .*ModelIndex and update exhaustively.


1341-1345: Variant correctly wired into the top-level Event enum – looks good

crates/dojo/core/src/world/storage.cairo (4)

298-307: Ohayo! The schema indexing change looks correct sensei!

The update from ModelIndex::Id to ModelIndex::Schema properly aligns with the new schema-based read operations.


309-323: Perfect consistency with the schema pattern sensei!

Using ptrs.to_schemas() maintains the same indexing approach as the singular read_schema method.


325-335: Ohayo! Clean implementation of schema writing sensei!

The method properly mirrors the read operation and maintains consistency with the existing storage patterns. Good use of the Introspect trait for layout determination.


337-352: Solid batch writing implementation sensei!

The method correctly handles multiple schemas and maintains consistency with the existing batch operation patterns in the codebase.

crates/dojo/core/src/world/world_contract.cairo (6)

46-47: Ohayo! Necessary imports for schema operations sensei!

These trait imports are required for the new schema handling functionality.


79-79: Event enum properly extended sensei!

The new event type follows the established naming convention and completes the schema update functionality.


206-215: Ohayo! Well-structured event for batch updates sensei!

The event properly captures all necessary information for schema-based batch member updates with appropriate key fields for filtering.


1318-1331: Excellent schema handling implementation sensei!

The code properly extracts struct fields, writes them to storage, and emits the appropriate batch update event. This enables the partial model update functionality as intended.


1356-1356: Ohayo! Proper schema deletion prevention sensei!

Correctly prevents schema entity deletion with a specific error, maintaining the integrity of partial update operations.


1375-1378: Clean pattern matching sensei!

Combining the Id and Schema cases reduces duplication since both read operations use the same underlying storage mechanism.

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thank you @bengineer42 for the work on that.

Ideally we want to target the new proc macro and not the builtin plugin. I should merge that on main today, this will require a rebase on your end. 🫡

Will make an other functional review right after.

@glihm
Copy link
Collaborator

glihm commented Jul 14, 2025

proc macros have been merged, could you rebase please? Don't hesitate to reach out if you have any questions about the proc macros.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
crates/dojo/core-tests/src/tests/model/model.cairo (1)

394-407: Great test coverage for the schema write functionality, sensei!

The test correctly exercises the new write_schema functionality by:

  1. Creating a full model and writing it to storage
  2. Creating a partial schema with only v0 and v3 fields
  3. Applying the schema update using world.write_schema
  4. Verifying that only the specified fields were updated

The test logic is sound and follows the established patterns in the file.

Consider adding additional test cases to improve coverage:

+#[test]
+fn test_write_schema_multiple_models() {
+    let mut world = spawn_foo_world();
+    let foo1 = Foo4 { id: 1, v0: 2, v1: 3, v2: 4, v3: AStruct { a: 5, b: 6, c: 7, d: 8 } };
+    let foo2 = Foo4 { id: 2, v0: 12, v1: 13, v2: 14, v3: AStruct { a: 15, b: 16, c: 17, d: 18 } };
+    world.write_models([@foo1, @foo2].span());
+    
+    let schema = FooSchema { v0: 100, v3: AStruct { a: 101, b: 102, c: 103, d: 104 } };
+    world.write_schemas([foo1.ptr(), foo2.ptr()].span(), [@schema, @schema].span());
+    
+    let model1: Foo4 = world.read_model(foo1.id);
+    let model2: Foo4 = world.read_model(foo2.id);
+    
+    assert_eq!(model1.v0, 100);
+    assert_eq!(model1.v1, foo1.v1); // unchanged
+    assert_eq!(model2.v0, 100);
+    assert_eq!(model2.v1, foo2.v1); // unchanged
+}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b52d44 and 56e94ca.

📒 Files selected for processing (12)
  • bin/sozo/src/commands/events.rs (1 hunks)
  • crates/dojo/core-tests/src/tests/model/model.cairo (4 hunks)
  • crates/dojo/core/src/lib.cairo (1 hunks)
  • crates/dojo/core/src/meta/layout.cairo (2 hunks)
  • crates/dojo/core/src/model/definition.cairo (1 hunks)
  • crates/dojo/core/src/model/model.cairo (2 hunks)
  • crates/dojo/core/src/model/storage.cairo (1 hunks)
  • crates/dojo/core/src/world/errors.cairo (1 hunks)
  • crates/dojo/core/src/world/storage.cairo (2 hunks)
  • crates/dojo/core/src/world/world_contract.cairo (6 hunks)
  • crates/dojo/world/src/contracts/abigen/model.rs (16 hunks)
  • crates/dojo/world/src/contracts/abigen/world.rs (106 hunks)
✅ Files skipped from review due to trivial changes (1)
  • crates/dojo/core/src/meta/layout.cairo
🚧 Files skipped from review as they are similar to previous changes (9)
  • crates/dojo/core/src/world/errors.cairo
  • crates/dojo/core/src/model/definition.cairo
  • crates/dojo/core/src/model/model.cairo
  • crates/dojo/core/src/lib.cairo
  • crates/dojo/core/src/model/storage.cairo
  • bin/sozo/src/commands/events.rs
  • crates/dojo/core/src/world/world_contract.cairo
  • crates/dojo/core/src/world/storage.cairo
  • crates/dojo/world/src/contracts/abigen/world.rs
🧰 Additional context used
🧠 Learnings (2)
crates/dojo/core-tests/src/tests/model/model.cairo (2)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in `crates/dojo/world/src/contracts/abigen/` is auto-generated and should be excluded from code reviews.
crates/dojo/world/src/contracts/abigen/model.rs (2)
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/world/src/contracts/abigen/world.rs:3809-0
Timestamp: 2024-11-05T04:30:12.852Z
Learning: Code in `crates/dojo/world/src/contracts/abigen/` is auto-generated and should be excluded from code reviews.
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
🔇 Additional comments (2)
crates/dojo/world/src/contracts/abigen/model.rs (1)

1-905: Ohayo sensei! This auto-generated file should be excluded from review.

Per the retrieved learnings and the explicit header notice, code in crates/dojo/world/src/contracts/abigen/ is auto-generated by cainome and should be excluded from code reviews. The formatting changes shown are automatically applied by the code generation tooling.

crates/dojo/core-tests/src/tests/model/model.cairo (1)

40-40: Ohayo! LGTM on the PartialEq additions, sensei!

The PartialEq derivations are correctly added to enable equality comparisons in the new test function. This follows the established pattern in the codebase and is necessary for the assert_eq! macro to work.

Also applies to: 49-49, 59-59

- Removed the favorite_item parameter from the update_player_config_items function in both the trait and implementation.
- Updated the implementation to determine the favorite_item based on the length of the items array, defaulting to the last item if available.
Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Let's see if we want this reworked on the new proc macros + the adaptation for Torii to ensure this can work. 👍

@glihm glihm marked this pull request as draft September 9, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants