-
Notifications
You must be signed in to change notification settings - Fork 206
Add write schema 1.5 #3273
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
base: main
Are you sure you want to change the base?
Add write schema 1.5 #3273
Conversation
- 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.
WalkthroughOhayo sensei! This update introduces schema-based batch operations for models, new event handling for Changes
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)
sequenceDiagram
participant User
participant Contract
User->>Contract: delete_entity_internal(ModelIndex::Schema)
Contract->>Contract: panic(DELETE_ENTITY_SCHEMA)
Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (7)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
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
♻️ 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 singularStoreUpdateMember
event.format!("Store update member ({})", tag), + format!("Store update members ({})", tag),
examples/spawn-and-move/manifest_dev.json (2)
1168-1194
: Event layout: consider promotingmember_selectors
to akey
for efficient filtering
StoreUpdateMembers
only usesselector
andentity_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/CLIThe 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
⛔ 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 ofPartialEq
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 ofPlayerConfig
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 existingto_indexes
method while using theModelIndex::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
andLayoutTrait
while removing the obsoleteLayoutCompareTrait
. 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:
- Write initial model
- Create schema with updated fields
- Use
write_schema
to update only specific members- 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
toLayoutTrait
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 itsselectors
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 everymatch
onModelIndex
now thatSchema
existsAdding the
Schema
variant is legit, but anymatch
orfrom_repr()
logic that enumeratesModelIndex
will panic if it doesn’t include this arm. Grep the codebase formatch .*ModelIndex
and update exhaustively.
1341-1345
: Variant correctly wired into the top-levelEvent
enum – looks goodcrates/dojo/core/src/world/storage.cairo (4)
298-307
: Ohayo! The schema indexing change looks correct sensei!The update from
ModelIndex::Id
toModelIndex::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 singularread_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
andSchema
cases reduces duplication since both read operations use the same underlying storage mechanism.
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.
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.
proc macros have been merged, could you rebase please? Don't hesitate to reach out if you have any questions about the proc macros. |
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
🧹 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:
- Creating a full model and writing it to storage
- Creating a partial schema with only
v0
andv3
fields- Applying the schema update using
world.write_schema
- 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
📒 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 theassert_eq!
macro to work.Also applies to: 49-49, 59-59
…nd favorite_item option
- 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.
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.
Let's see if we want this reworked on the new proc macros + the adaptation for Torii to ensure this can work. 👍
Description
Added the write schema method to dojo to write part of a model efficiently
Related issue
Tests
Added to documentation?
Checklist
scripts/rust_fmt.sh
,scripts/cairo_fmt.sh
)scripts/clippy.sh
,scripts/docs.sh
)Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Style