feat(core): model get/set functions #2159
Conversation
WalkthroughThe changes streamline entity manipulation in the Dojo framework by simplifying function calls and introducing new abstractions. Specifically, macros for entity deletion, retrieval, and setting have been revised for clarity and efficiency. Key updates include renaming parameters, adding new enums, and refining event structures to improve code readability and functionality. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
remybar
force-pushed
the
feat-get_by_entity_id
branch
6 times, most recently
from
3b0e48d to
52051d4
Compare
There was a problem hiding this comment.
@remybar I've talked with @lambda-0x and we could actually modify the macro set!, get! and delete! to actually use those new method instead of generating code.
Can definitively be done in an other PR if you want to. Was to let this info here or the record. 👍
There was a problem hiding this comment.
Thanks for the rework here @remybar!
I feel like we could refactor the whole set_entity/by_id/member in only one function to make the API clearer and a bit more compact and comprehensive for future composition on it.
It can be done in this PR, or later if you feel more comfortable to merge a first iteration. Also, we should refactor the core by moving away all the functions with _ to have a dedicated logic module for those one.
Here's my quick analysis:
model_id (being model_selector if we keep the name we have in IModel) -> we always need this one.
keys or entity_id -> we could have an enum to identify the "index"/"key clause"?
layout -> still the same anyway as we search for the member layout into the full layout of the model. But I think it will be more flexible in the future to actually send only the layout we're interested it, instead of the full layout every time. We could extract the layout before calling the world instead of doing it inside the world.
values -> we don't care much here, it still a Span<felt252> applicable to both member/values/full.
enum ModelIndex {
Keys(Span<felt252>),
Id(felt252),
}
enum ModelValue {
Full(Span<felt252>),
// (selector, serialized value)
Member((felt252, Span<felt252>)),
}
// This should work for all:
// - set entity with keys / id
// - set entity values full / member
// - any combination of the above
fn set_entity(
model_selector: felt252,
index: ModelIndex,
values: ModelValue,
layout: Layout,
) { ... }
remybar
force-pushed
the
feat-get_by_entity_id
branch
2 times, most recently
from
ff41042 to
fdd32ce
Compare
Yes, sure ! |
remybar
force-pushed
the
feat-get_by_entity_id
branch
from
fdd32ce to
298479d
Compare
remybar
force-pushed
the
feat-get_by_entity_id
branch
6 times, most recently
from
4cb2d9f to
5bf5b1f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Outside diff range, codebase verification and nitpick comments (3)
crates/torii/types-test/src/contracts.cairo (1)
Line range hint
45-45:
Suggestion: Add error handling fortry_intoconversions.The
try_intoconversions should have error handling to manage potential conversion failures.- let random_u8 = random(pedersen::pedersen(seed(), record_idx.into()), 0, 100) - .try_into() - .unwrap(); + let random_u8 = match random(pedersen::pedersen(seed(), record_idx.into()), 0, 100).try_into() { + Ok(value) => value, + Err(e) => { + // Handle error (e.g., log it, return from function, etc.) + return; + } + };crates/dojo-core/src/world_test.cairo (2)
25-25: Remove Unused ImportThe
entity_id_from_keysimport is not used anywhere in the file.- use dojo::utils::entity_id_from_keys;
Line range hint
1368-1373:
Add assertion for expected panic messageThe function
test_set_entity_with_unexpected_array_model_layoutis correct. Adding an assertion to check the expected panic message would improve the test coverage.+ #[should_panic(expected = "Unexpected layout type for a model.")]
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (39)
- crates/dojo-core/src/base_test.cairo (1 hunks)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (25 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (8 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (5 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (8 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (2 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (16 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review due to trivial changes (8)
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- crates/sozo/ops/src/events.rs
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
Additional comments not posted (99)
examples/spawn-and-move/src/mock_token.cairo (1)
5-8: Verify the usage ofset!macro.Ensure that the
set!macro is compatible with the newget/setmethods introduced in this PR.examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1)
2-3: Confirm the correctness of the updated class hashes.Ensure that the new class hashes are correct and consistent with the rest of the configuration.
Verification successful
The updated class hashes are consistent across multiple configuration files.
The class hashes in the reviewed file match the values found in other configuration files, indicating consistency across different environments.
examples/spawn-and-move/manifests/dev/deployment/manifest.tomlexamples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.tomlexamples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.tomlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the updated class hashes. # Test: Search for the new class hashes in the configuration files. rg --type toml 'class_hash = "0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028"'Length of output: 1107
crates/dojo-core/src/lib.cairo (1)
10-11: LGTM! The addition of themodel_testmodule is appropriate.The
model_testmodule has been correctly added under thecfg(test)attribute.crates/dojo-core/src/utils.cairo (1)
17-19: LGTM! Theentity_id_from_keysfunction is correct and efficient.The function computes an entity ID from the given keys using a poseidon hash.
examples/spawn-and-move/Scarb.toml (1)
38-56: LGTM! The added configurations for the release profile are consistent with the dev profile.The changes ensure that the release profile has similar settings, which is crucial for maintaining consistency across environments.
crates/dojo-core/src/model.cairo (2)
1-1: LGTM! The added import statements are necessary for the new implementations.The imports for
ModelIndexare required for the new trait implementations introduced in this file.
4-16: LGTM! TheModelEntitytrait is well-defined and enhances modularity.The trait introduces necessary methods for handling model entities, aligning with the overall architecture.
crates/dojo-world/src/contracts/naming.rs (1)
60-61: LGTM! Thecompute_bytearray_hashfunction has been updated for better clarity.The parameter name change from
namespacetovaluealigns with the naming conventions and improves clarity.crates/dojo-lang/src/semantics/test_data/set (1)
122-136: LGTM! The updated test case ensures thesetmacro works as expected.The changes ensure that the
setmacro works as expected with the new implementation.crates/torii/types-test/src/contracts.cairo (2)
13-13: Note: Usage ofset!macro.Consider replacing the
set!macro with the newsetmethod in future updates.
Line range hint
45-45:
Note: Usage ofdelete!macro.Consider replacing the
delete!macro with the newdeletemethod in future updates.crates/dojo-core/src/model_test.cairo (13)
21-24: LGTM!The test function correctly verifies the ID of
FooEntity.
27-33: LGTM!The test function correctly verifies the values of
FooEntity.
36-41: LGTM!The test function correctly verifies the creation of
FooEntityfrom values.
44-48: LGTM!The test function correctly verifies the handling of bad data.
51-69: LGTM!The test function correctly verifies the retrieval and update of
FooEntity.
72-85: LGTM!The test function correctly verifies the deletion of
FooEntity.
88-107: LGTM!The test function correctly verifies the retrieval and update of a member of
FooEntity.
110-125: LGTM!The test function correctly verifies the retrieval and update of a field of
FooEntity.
128-143: LGTM!The test function correctly verifies the retrieval and update of a
Foomodel.
146-161: LGTM!The test function correctly verifies the deletion of a
Foomodel.
164-180: LGTM!The test function correctly verifies the retrieval and update of a member of a
Foomodel.
183-197: LGTM!The test function correctly verifies the retrieval and update of a field of a
Foomodel.
183-197: Duplicate: LGTM!The test function correctly verifies the retrieval and update of a field of a
Foomodel.crates/dojo-lang/src/inline_macros/delete.rs (1)
148-150: Note: Usage ofdeletemethod.The
deletemethod is used to delete the models. Ensure that all models support this method.crates/dojo-lang/src/semantics/test_data/get (3)
Line range hint
12-12:
LGTM!The test case correctly verifies the behavior of the
getmacro with no parameters.
Line range hint
12-12:
LGTM!The test case correctly verifies the behavior of the
getmacro with incorrect parameters.
204-204: LGTM!The test case correctly verifies the behavior of the
getmacro with correct parameters.crates/dojo-lang/src/inline_macros/set.rs (1)
163-164: Ensure correct usage ofdojo::model::Model::set.The lines add code to set model instances using
dojo::model::Model::set. Verify thatdojo::model::Model::setis correctly implemented and used.Verification successful
Ensure correct usage of
dojo::model::Model::set.The
dojo::model::Model::setmethod is correctly implemented and used within thecrates/dojo-lang/src/model.rsfile. The function integrates properly with thedojoframework to set model instances.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `dojo::model::Model::set`. # Test: Search for the `dojo::model::Model::set` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 $'impl Model for $_ { fn set($_, $_) { $$$ } }'Length of output: 383
Script:
#!/bin/bash # Description: Verify the usage of `dojo::model::Model::set`. # Test: Search for the `dojo::model::Model::set` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 'impl Model for' | rg 'fn set'Length of output: 52
Script:
#!/bin/bash # Description: Verify the usage of `dojo::model::Model::set`. # Test: Search for the `dojo::model::Model::set` implementation using ast-grep. Expect: Correct implementation and usage. ast-grep --lang rust --pattern $'impl Model for $_ { fn set($_, $_) { $$$ } }'Length of output: 87
Script:
#!/bin/bash # Description: Broaden the search criteria to locate the `set` method within the `Model` trait. # Search for the `Model` trait definition. rg --type rust 'trait Model' -A 10 # Search for any `set` method definitions. rg --type rust 'fn set' -A 5Length of output: 35576
Script:
#!/bin/bash # Description: Investigate the `fn set` method in `crates/dojo-lang/src/model.rs`. # Show the surrounding lines of the `fn set` method in `crates/dojo-lang/src/model.rs`. rg --type rust 'fn set' -A 10 crates/dojo-lang/src/model.rsLength of output: 1968
crates/dojo-core/src/base_test.cairo (1)
181-181: Verify the pre-computed address.Ensure the pre-computed address
0x3f18f40f6d815f380d1ea8278289b58f076abf6e396d7a88eed5d152da8190bis correct and consistent with the expected address.crates/dojo-core/src/resource_metadata.cairo (6)
127-128: Ensure correct computation of the entity ID.The method computes the entity ID for a
ResourceMetadataentity. Verify that the computation is correctly implemented and used.Verification successful
Ensure correct computation of the entity ID.
The method
entity_idincrates/dojo-lang/src/model.rscorrectly computes the entity ID usingcore::poseidon::poseidon_hash_span(self.keys()).
crates/dojo-lang/src/model.rs: fn entity_id(self: @$type_name$) -> felt252 {crates/dojo-lang/src/model.rs: core::poseidon::poseidon_hash_span(self.keys())Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the computation of the entity ID in `ResourceMetadataModel`. # Test: Search for the `entity_id` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 $'fn entity_id($_) { $$$ }'Length of output: 325
Script:
#!/bin/bash # Description: Verify the computation of the entity ID in `ResourceMetadataModel`. # Test: Search for the `entity_id` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 'fn entity_id\(self: @ResourceMetadata\) -> felt252 {'Length of output: 244
Script:
#!/bin/bash # Description: Verify the computation of the entity ID in `ResourceMetadataModel`. # Test: Search for the `entity_id` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 'fn entity_id'Length of output: 801
51-53: Ensure correct usage ofModelIndex::Keys.The method deletes a
ResourceMetadataentity using theModelIndex::Keysvariant. Verify thatModelIndex::Keysis correctly implemented and used.Verification successful
Verified correct usage of
ModelIndex::Keys.The method
deleteinResourceMetadatacorrectly usesModelIndex::Keys. TheModelIndexenum, including theKeysvariant, is defined and used consistently across multiple files in the codebase.
crates/dojo-core/src/world.cairo: Enum definition and usage ofModelIndexand its variants.crates/dojo-core/src/resource_metadata.cairo: Usage ofModelIndex::Keys.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `ModelIndex::Keys` in `ResourceMetadataModel`. # Test: Search for the `ModelIndex::Keys` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 $'enum ModelIndex { Keys($_), $$$ }'Length of output: 350
Script:
#!/bin/bash # Description: Verify the usage of `ModelIndex::Keys` in `ResourceMetadataModel`. # Test: Search for the `ModelIndex::Keys` implementation. Expect: Correct implementation and usage. ast-grep --lang rust --pattern $'enum ModelIndex { Keys($_), $$$ }'Length of output: 72
Script:
#!/bin/bash # Description: Broaden the search to find any occurrence of `ModelIndex` in the codebase. # Test: Search for any occurrence of `ModelIndex`. Expect: Find the definition or usage of `ModelIndex`. rg 'ModelIndex'Length of output: 35822
55-68: Ensure correct usage ofModelIndex::MemberId.The method retrieves a member of a
ResourceMetadataentity using theModelIndex::MemberIdvariant. Verify thatModelIndex::MemberIdis correctly implemented and used.
45-49: Ensure correct usage ofModelIndex::Keys.The method sets a
ResourceMetadataentity using theModelIndex::Keysvariant. Verify thatModelIndex::Keysis correctly implemented and used.Verification successful
Verified usage of
ModelIndex::Keys.The
ModelIndex::Keysvariant is correctly implemented and used within the codebase, specifically in the context of setting entities in thedojo::worldmodule.
crates/dojo-world/src/contracts/model.rscrates/dojo-world/src/contracts/abi/world.rscrates/dojo-lang/src/model.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `ModelIndex::Keys` in `ResourceMetadataModel`. # Test: Search for the `ModelIndex::Keys` implementation. Expect: Correct implementation and usage. rg --type rust -A 5 $'enum ModelIndex { Keys($_), $$$ }'Length of output: 350
Script:
#!/bin/bash # Description: Verify the usage of `ModelIndex::Keys` in `ResourceMetadataModel`. # Test: Search for the `ModelIndex::Keys` implementation. Expect: Correct implementation and usage. rg --type rust 'enum ModelIndex' -A 10Length of output: 40
Script:
#!/bin/bash # Description: Search for all enum definitions in the codebase and examine them for the presence of the `Keys` variant. # Test: Search for all enum definitions. Expect: Find the enum that contains the `Keys` variant. rg --type rust 'enum ' -A 10Length of output: 123878
Script:
#!/bin/bash # Description: Search for the definition and usage of `ModelIndex` and `Keys` in the codebase. # Test: Search for `ModelIndex` definition and its variants. rg --type rust 'ModelIndex' -A 10 # Test: Search for `Keys` variant usage. rg --type rust 'Keys' -A 10Length of output: 83618
70-88: Ensure correct usage ofModelIndex::MemberId.The method sets a member of a
ResourceMetadataentity using theModelIndex::MemberIdvariant. Verify thatModelIndex::MemberIdis correctly implemented and used.
Line range hint
28-44:
Ensure correct usage ofModelIndex::Keys.The method retrieves a
ResourceMetadataentity using theModelIndex::Keysvariant. Verify thatModelIndex::Keysis correctly implemented and used.examples/spawn-and-move/manifests/dev/deployment/manifest.toml (4)
3-7: Verify the updated class hash and address.Ensure the updated class hash
0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7eand address0x6d95c696211b4453e1f4b1b2dc9fb9d91602d4a7a1550567362bd9b8bcd9a82are correct and consistent with the expected values.Verification successful
The class hash and address have been verified and are consistent across multiple files.
class_hash = "0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e"address = "0x6d95c696211b4453e1f4b1b2dc9fb9d91602d4a7a1550567362bd9b8bcd9a82"These values are correctly updated and consistent throughout the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated class hash and address in `[world]` section. # Test: Search for the class hash and address usage. Expect: Correct and consistent values. rg --type toml -A 5 $'class_hash = "0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e"' rg --type toml -A 5 $'address = "0x6d95c696211b4453e1f4b1b2dc9fb9d91602d4a7a1550567362bd9b8bcd9a82"'Length of output: 4519
26-28: Verify the updated class hash and address.Ensure the updated class hash
0x288e853c84bde01bdbfbd8b8ffe0ff91ebbce90891e5d300f3395abbb50094dand address0x1a0504af4ffec1a748ae115e8d36b8cd7913298308ae4eb39e1eee8fc19d516are correct and consistent with the expected values.
43-45: Verify the updated class hash and address.Ensure the updated class hash
0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028and address0x5a8f0aacc5fa9b30fdffee908d7e6093dedda92ad6deefff31aa06b473312e8are correct and consistent with the expected values.Verification successful
Verification Successful: Consistent Class Hash and Address
The class hash
0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028and address0x5a8f0aacc5fa9b30fdffee908d7e6093dedda92ad6deefff31aa06b473312e8are consistent and correctly updated across the relevant manifests.
examples/spawn-and-move/manifests/dev/deployment/manifest.tomlexamples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.tomlexamples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.tomlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated class hash and address in `[[contracts]]` section for `dojo_examples-mock_token`. # Test: Search for the class hash and address usage. Expect: Correct and consistent values. rg --type toml -A 5 $'class_hash = "0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028"' rg --type toml -A 5 $'address = "0x5a8f0aacc5fa9b30fdffee908d7e6093dedda92ad6deefff31aa06b473312e8"'Length of output: 3778
Line range hint
57-62:
Verify the new contract's class hash and address.Ensure the new contract's class hash
0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7and address0x5958b21ca9debb6bf444aebefef92305d3b0f3fd39e20097161282078666873are correct and consistent with the expected values.#!/bin/bash # Description: Verify the new contract's class hash and address in `[[contracts]]` section for `dojo_examples-others`. # Test: Search for the class hash and address usage. Expect: Correct and consistent values. rg --type toml -A 5 $'class <details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_4/4 section_4/5] Review section `[[contracts]]` for `dojo_examples-others`. [observations] The class hash `0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7` and address `0x5958b21ca9debb6bf444aebefef92305d3b0f3fd39e20097161282078666873` were found in the expected manifest files. [analysis] The class hash and address are correctly used and consistent in the repository. The verification confirms their presence and correctness in the relevant manifest files. [actions] Generate the final response indicating successful verification. --> **Verification successful: Class hash and address are consistent.** The new contract's class hash `0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7` and address `0x5958b21ca9debb6bf444aebefef92305d3b0f3fd39e20097161282078666873` are correctly used and consistent within the repository. - Found in `examples/spawn-and-move/manifests/dev/deployment/manifest.toml` - Found in `examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-others-61de2c18.toml` - Found in `examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml` </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify the new contract's class hash and address in `[[contracts]]` section for `dojo_examples-others`. # Searching for the class hash usage in the repository to ensure it is consistent. rg --type toml --word-boundary '0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7' # Searching for the address usage in the repository to ensure it is consistent. rg --type toml --word-boundary '0x5958b21ca9debb6bf444aebefef92305d3b0f3fd39e20097161282078666873'Length of output: 1091
Script:
#!/bin/bash # Description: Verify the new contract's class hash and address in `[[contracts]]` section for `dojo_examples-others`. # Searching for the class hash usage in the repository to ensure it is consistent. rg --type toml --word-regexp '0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7' # Searching for the address usage in the repository to ensure it is consistent. rg --type toml --word-regexp '0x5958b21ca9debb6bf444aebefef92305d3b0f3fd39e20097161282078666873'Length of output: 1333
crates/dojo-world/src/contracts/model.rs (1)
132-136: Verify updated function call.The
entityfunction call now usesabigen::world::ModelIndex::Keys. Ensure that theentityfunction is updated to handle the new parameter correctly.Verification successful
Let's re-run the script to search for the
fn entityfunction implementation across all files and include additional lines for context.
Verification successful.
The
entityfunction incrates/dojo-core/src/world.cairohandles theModelIndexparameter correctly, including theModelIndex::Keysvariant.
crates/dojo-core/src/world.cairo: Theentityfunction implementation correctly processes theModelIndex::Keysvariant.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `entity` function implementation in the codebase. # Test: Search for the `entity` function implementation. Expect: The function to handle the `ModelIndex` parameter correctly. rg --type cairo -A 10 'fn entity'Length of output: 65
Script:
#!/bin/bash # Description: Verify the `entity` function implementation in the codebase. # Test: Search for the `entity` function implementation. Expect: The function to handle the `ModelIndex` parameter correctly. rg -A 10 'fn entity'Length of output: 64937
examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (2)
289-300: Verify new function implementation.The
update_player_name_valuefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.Verification successful
The
update_player_name_valuefunction is implemented correctly.The function
update_player_name_valueis defined inexamples/spawn-and-move/src/actions.cairoand includes the necessary steps to update the player's name and verify the change.
- Location:
examples/spawn-and-move/src/actions.cairo- Implementation Details:
- Signature:
fn update_player_name_value(ref world: IWorldDispatcher, name: ByteArray)- Retrieves the caller address.
- Gets the player's configuration.
- Sets the player's name.
- Verifies the change.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `update_player_name_value` function implementation in the codebase. # Test: Search for the `update_player_name_value` function implementation. Expect: The function to be implemented correctly. rg --type cairo -A 10 'fn update_player_name_value'Length of output: 83
Script:
#!/bin/bash # Description: Verify the `update_player_name_value` function implementation in the codebase. # Test: Search for the `update_player_name_value` function implementation. Expect: The function to be implemented correctly. rg 'fn update_player_name_value' -A 10Length of output: 1910
277-288: Verify new function implementation.The
update_player_namefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.Verification successful
The
update_player_namefunction is correctly implemented.The function is defined in
examples/spawn-and-move/src/actions.cairoand matches the expected behavior described in the review comment. It takes anameparameter of typeByteArrayand ensures the player's name is updated correctly.
examples/spawn-and-move/src/actions.cairoScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `update_player_name` function implementation in the codebase. # Test: Search for the `update_player_name` function implementation. Expect: The function to be implemented correctly. rg --type cairo -A 10 'fn update_player_name'Length of output: 77
Script:
#!/bin/bash # Description: Verify the `update_player_name` function implementation in the codebase. # Find all Cairo files in the repository fd --extension cairo > cairo_files.txt # Search for the `update_player_name` function within the found Cairo files rg 'fn update_player_name' -A 10 -f cairo_files.txtLength of output: 138
Script:
#!/bin/bash # Description: Verify the `update_player_name` function implementation in the codebase. # Find all Cairo files and search for the `update_player_name` function within them fd --extension cairo | xargs rg 'fn update_player_name' -A 10Length of output: 2811
examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (2)
277-288: Verify new function implementation.The
update_player_namefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.
289-300: Verify new function implementation.The
update_player_name_valuefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.
examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (3)
277-288: Verify new function implementation.The
update_player_namefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.
289-300: Verify new function implementation.The
update_player_name_valuefunction takes anameparameter of typecore::byte_array::ByteArray. Ensure that the function is implemented correctly in the codebase.
324-334: Verify new function implementation.The
call_somethingfunction takes asomething_addressparameter of typecore::starknet::contract_address::ContractAddress. Ensure that the function is implemented correctly in the codebase.Verification successful
Function
call_somethingImplementation VerifiedThe
call_somethingfunction has been implemented correctly in the codebase. It takes aContractAddressas a parameter and uses it to create anISomethingDispatcherinstance, which then calls thesomethingmethod.
- Location:
examples/spawn-and-move/src/actions.cairoScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `call_something` function implementation in the codebase. # Test: Search for the `call_something` function implementation. Expect: The function to be implemented correctly. rg --type cairo -A 10 'fn call_something'Length of output: 73
Script:
#!/bin/bash # Description: Verify the `call_something` function implementation in the codebase. # Step 1: Find all .cairo files in the repository fd -e cairo > cairo_files.txt # Step 2: Search for the `call_something` function implementation in the found .cairo files while read -r file; do echo "Searching in $file" rg 'fn call_something' "$file" -A 10 done < cairo_files.txtLength of output: 13429
examples/spawn-and-move/src/actions.cairo (5)
11-12: New functions added to IActions trait.The new functions
update_player_nameandupdate_player_name_valueare consistent with the trait's purpose and follow the correct syntax.
85-106: Correctness and Consistency Check for New Lines inmoveFunction.The new lines correctly replace the usage of the
get!andset!macros with the new methods for getting and setting model and entity values. The logic and syntax are correct and consistent with the rest of the function.
125-129: Correctness and Consistency Check for New Lines inreset_player_configFunction.The new lines correctly replace the usage of the
get!anddelete!macros with the new methods for getting and deleting model and entity values. The logic and syntax are correct and consistent with the rest of the function.
162-180: Correctness and Consistency Check for New Functionsupdate_player_nameandupdate_player_name_value.The new functions
update_player_nameandupdate_player_name_valueare correctly implemented. They follow the correct logic and syntax for getting and setting player configuration values. The assertions ensure the correctness of the updated values.
Line range hint
233-249:
Correctness and Completeness Check for New Lines intest_moveFunction.The new lines for asserting the correctness of the player's position are correctly implemented. The assertions ensure that the player's position is updated as expected after the move action.
crates/dojo-lang/src/introspect/layout.rs (1)
30-30: Correctness and Consistency Check for New Line inbuild_field_layoutsFunction.The new line for getting the field selector from the field name using
get_selector_from_nameis correctly implemented. It follows the correct logic and syntax for generating the field layout.
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (6)
49-66: New Enum Definition:ModelIndex.The new enum
ModelIndexwith variantsKeys,Id, andMemberIdis correctly defined. The structure and types are consistent with the rest of the file.
303-308: Function Parameter Changes:entity.The changes to the
entityfunction parameters are correctly implemented. The new parametersmodel_selectorandindexare consistent with the newModelIndexenum.
327-332: Function Parameter Changes:set_entity.The changes to the
set_entityfunction parameters are correctly implemented. The new parametersmodel_selectorandindexare consistent with the newModelIndexenum.
351-356: Function Parameter Changes:delete_entity.The changes to the
delete_entityfunction parameters are correctly implemented. The new parametersmodel_selectorandindexare consistent with the newModelIndexenum.
Line range hint
922-936:
Event Changes:StoreUpdateRecord.The changes to the
StoreUpdateRecordevent are correctly implemented. The new membersentity_idandvaluesare consistent with the event's purpose and structure.
942-957: New Event Definition:StoreDelRecord.The new event
StoreDelRecordis correctly defined. The structure and members are consistent with the rest of the file.
examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (5)
49-66: LGTM: Addition ofModelIndexenum.The
ModelIndexenum is well-defined and provides a clear way to identify models using different types of identifiers.
303-308: LGTM: Modification toentityfunction parameters.The change aligns with the new
ModelIndexenum and improves clarity by usingmodel_selectorandindexinstead ofmodelandkeys.
327-332: LGTM: Modification toset_entityfunction parameters.The change aligns with the new
ModelIndexenum and improves clarity by usingmodel_selectorandindexinstead ofmodelandkeys.
351-356: LGTM: Modification todelete_entityfunction parameters.The change aligns with the new
ModelIndexenum and improves clarity by usingmodel_selectorandindexinstead ofmodelandkeys.
Line range hint
922-958: LGTM: Addition ofStoreUpdateRecordandStoreDelRecordevents.The events are well-defined and provide necessary information for store updates and deletions.
examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (4)
49-66: Addition ofModelIndexenum is correct.The enum
ModelIndexwith variantsKeys,Id, andMemberIdis well-defined and uses appropriate types.
303-308: Parameter updates forentityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
327-332: Parameter updates forset_entityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
351-356: Parameter updates fordelete_entityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (4)
49-66: Addition ofModelIndexenum is correct.The enum
ModelIndexwith variantsKeys,Id, andMemberIdis well-defined and uses appropriate types.
303-308: Parameter updates forentityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
327-332: Parameter updates forset_entityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
351-356: Parameter updates fordelete_entityfunction are correct.The parameters
model_selectorandindexof typeModelIndexare consistent with the new enum definition.
crates/dojo-world/src/contracts/abi/world.rs (4)
55-72: New EnumModelIndexis well-defined.The
ModelIndexenum with variantsKeys,Id, andMemberIdis well-structured and aligns with the PR objectives of enhancing entity identification and manipulation.
309-314: Parameter changes inentityfunction enhance clarity.The updated parameters
model_selectorandindeximprove the clarity and functionality of theentityfunction.
333-338: Parameter changes inset_entityfunction are consistent and clear.The updated parameters
model_selectorandindexenhance the clarity and maintain consistency with the refactoring goals.
357-362: Parameter changes indelete_entityfunction align with refactoring goals.The updated parameters
model_selectorandindeximprove the clarity and maintain consistency with the refactoring goals.
crates/dojo-lang/src/model.rs (3)
242-370: New methods for handling model structs are well-structured.The new methods for handling model parameters, generating field accessors, and managing model entities are well-structured and align with the PR objectives.
Line range hint
443-493: Refactoredget,set, anddeletemethods enhance clarity and maintainability.The refactoring of the
get,set, anddeletemethods to use the newModelIndexenum streamlines operations and improves clarity.
495-532: Newget_memberandset_membermethods are efficient and clear.The new
get_memberandset_membermethods provide a clear and efficient way to handle individual field operations.
crates/dojo-core/src/world.cairo (9)
130-130: LGTM! Verify the usage ofentity_id_from_keys.The
entity_id_from_keysutility function appears to be useful for generating entity IDs from keys.However, ensure that this function is used correctly throughout the codebase.
Verification successful
Verified: Usage of
entity_id_from_keysThe
entity_id_from_keysutility function is used correctly across various parts of the codebase, as observed in the following locations:
MovesTrait::entity_id_from_keys(player)PlayerConfigTrait::entity_id_from_keys(player)- Multiple direct usages within
.cairofiles without any apparent issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `entity_id_from_keys`. # Test: Search for all usages of `entity_id_from_keys` in the codebase. rg --type cairo -A 5 $'entity_id_from_keys'Length of output: 72
Script:
#!/bin/bash # Description: Verify the usage of `entity_id_from_keys`. # Find all .cairo files and search for usages of `entity_id_from_keys` within them. fd -e cairo -x rg 'entity_id_from_keys' {}Length of output: 764
236-237: LGTM! Verify the event emission ofStoreDelRecord.The
StoreDelRecordevent struct appears to be necessary for deleting store records.However, ensure that this event is emitted correctly throughout the codebase.
Verification successful
Event Emission Verified:
StoreDelRecordThe
StoreDelRecordevent is correctly referenced and emitted in the codebase, including theworld.cairofile.
crates/dojo-core/src/world.cairo: Emission ofStoreDelRecord.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the event emission of `StoreDelRecord`. # Test: Search for all emissions of `StoreDelRecord` in the codebase. rg --type cairo -A 5 $'StoreDelRecord'Length of output: 67
Script:
#!/bin/bash # Description: Verify the event emission of `StoreDelRecord`. # Test: Search for all emissions of `StoreDelRecord` in the codebase without specifying file type. rg -A 5 'StoreDelRecord'Length of output: 27403
42-45: LGTM! Verify the function logic handlesModelIndexcorrectly.The changes to the
delete_entityfunction signature are consistent with the introduction of theModelIndexenum.However, ensure that the function's logic correctly handles the new
ModelIndexparameter.Verification successful
The function logic correctly handles the
ModelIndexparameter.The
delete_entityfunction implementation incrates/dojo-core/src/world.cairomatches on theModelIndexvariants and performs the appropriate actions, ensuring that theModelIndexparameter is correctly handled.
delete_entityimplementation incrates/dojo-core/src/world.cairolines 42-68Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Test: Search for the `delete_entity` function implementation. rg --type cairo -A 10 $'fn delete_entity'Length of output: 72
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Test: Search for the `delete_entity` function implementation in .cairo files. rg 'fn delete_entity' -t cairo -A 20Length of output: 68
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Test: Search for the `delete_entity` function implementation in all files. rg 'fn delete_entity' -A 50Length of output: 11198
6-12: LGTM! Verify consistent usage ofModelIndexenum.The
ModelIndexenum is well-defined and seems necessary for the new functionality.However, ensure that this enum is used consistently and correctly throughout the codebase.
Verification successful
LGTM! The
ModelIndexenum is used consistently across the codebase.The
ModelIndexenum is well-defined and consistently integrated throughout the codebase as observed in files such asworld.cairo,model.cairo,resource_metadata.cairo, andworld_test.cairo.
crates/dojo-core/src/world.cairocrates/dojo-core/src/model.cairocrates/dojo-core/src/resource_metadata.cairocrates/dojo-core/src/world_test.cairoScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of `ModelIndex` enum. # Test: Search for all usages of `ModelIndex` in the codebase. rg --type cairo -A 5 $'ModelIndex'Length of output: 63
Script:
#!/bin/bash # Description: Verify consistent usage of `ModelIndex` enum. # Test: Search for all usages of `ModelIndex` in the codebase. rg -A 5 $'ModelIndex'Length of output: 139927
36-37: LGTM! Verify the function logic handlesModelIndexcorrectly.The changes to the
set_entityfunction signature are consistent with the introduction of theModelIndexenum.However, ensure that the function's logic correctly handles the new
ModelIndexparameter.Verification successful
The function logic handles
ModelIndexcorrectly.The
set_entityfunction uses a match statement to handle different variants of theModelIndexenum (Keys,Id, andMemberId) and performs the appropriate actions for each variant.
- Keys: Calls
self._write_model_entityand emitsStoreSetRecord.- Id: Calls
self._write_model_entityand emitsStoreUpdateRecord.- MemberId: Calls
self._write_model_member.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Test: Search for the `set_entity` function implementation. rg --type cairo -A 10 $'fn set_entity'Length of output: 69
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Find all `.cairo` files and search for the `set_entity` function within them. fd -e cairo -x rg 'fn set_entity' -A 10 {}Length of output: 728
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly by gathering more lines from the implementation. # Find all `.cairo` files and search for the `set_entity` function within them, including more lines of context. fd -e cairo -x rg 'fn set_entity' -A 30 {}Length of output: 2765
753-833: LGTM! Verify the function logic for handlingModelIndex.The
entity,set_entity, anddelete_entityfunctions handle different cases ofModelIndexand perform corresponding operations.However, ensure that the logic for handling each case of
ModelIndexis correct.Verification successful
LGTM! Verify the function logic for handling
ModelIndex.The
entity,set_entity, anddelete_entityfunctions handle different cases ofModelIndexand perform corresponding operations.
- The
entityfunction correctly reads the entity or member based on theModelIndex.- The
set_entityfunction correctly writes the entity or member and emits the appropriate event.- The
delete_entityfunction correctly deletes the entity and emits the appropriate event or panics forMemberId.The logic for handling each case of
ModelIndexis correct.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function logic for handling `ModelIndex`. # Test: Search for the `entity`, `set_entity`, and `delete_entity` function implementations. rg --type cairo -A 10 $'fn entity' rg --type cairo -A 10 $'fn set_entity' rg --type cairo -A 10 $'fn delete_entity'Length of output: 208
Script:
#!/bin/bash # Description: Verify the function logic for handling `ModelIndex`. # Test: Search for the `entity`, `set_entity`, and `delete_entity` function implementations. rg -A 10 $'fn entity' rg -A 10 $'fn set_entity' rg -A 10 $'fn delete_entity'Length of output: 69360
27-32: LGTM! Verify the function logic handlesModelIndexcorrectly.The changes to the
entityfunction signature are consistent with the introduction of theModelIndexenum.However, ensure that the function's logic correctly handles the new
ModelIndexparameter.Verification successful
Verified: The function logic correctly handles
ModelIndex.The
entityfunction implementation inContractStatecorrectly handles theModelIndexparameter with appropriate logic for each variant (Keys,Id,MemberId).
ModelIndex::Keys(keys)computesentity_idfrom keys and reads the model entity.ModelIndex::Id(entity_id)reads the model entity directly.ModelIndex::MemberId((entity_id, member_id))reads the model member.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Test: Search for the `entity` function implementation. rg --type cairo -A 10 $'fn entity'Length of output: 65
Script:
#!/bin/bash # Description: Verify the function logic handles `ModelIndex` correctly. # Locate all .cairo files and search for the `entity` function implementation within them. fd -e cairo -x rg 'fn entity' {} -A 10Length of output: 1914
Script:
#!/bin/bash # Description: Extract the complete `entity` function implementation for verification. # Locate the .cairo file containing the `entity` function and extract the complete implementation. fd -e cairo -x rg 'fn entity' '{}' -A 20 -B 5Length of output: 4368
Line range hint
158-231:
LGTM! Verify the event emission ofStoreUpdateRecord.The
StoreUpdateRecordevent struct appears to be necessary for updating store records.However, ensure that this event is emitted correctly throughout the codebase.
62-62: LGTM! Verify the function logic and integration.The
can_write_modelfunction checks if a contract can write to a model.However, ensure that the function's logic is correct and that it integrates well with the rest of the codebase.
Verification successful
Let's correct the file type and search for the implementation of the
can_write_modelfunction.
To further verify the function logic and its integration, we need to examine the
_check_model_write_accessfunction and theself.resources.read()method, as well as theResourceDataenum.
To complete the verification, we need to check the implementations of
is_writer,is_account_owner,is_account_writer,_check_basic_write_access, and thereadmethod.
The function
can_write_modeland its associated methods are correctly implemented and integrated.The methods
is_writer,is_account_owner,is_account_writer, and_check_basic_write_accessare correctly checking various permissions required for writing to a model. TheResourceDataenum and the_check_model_write_accessfunction are also implemented as expected.
is_writer: Checks if a contract is a writer for a resource.is_account_owner: Checks if the calling account is the owner of a resource.is_account_writer: Checks if the calling account is a writer for a resource._check_basic_write_access: Combines the checks for writer, account owner, and account writer roles.ResourceDataenum: Defines various resource types including Model, Contract, Namespace, and None.The logic and integration for the
can_write_modelfunction appear to be correctly implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function logic and integration of `can_write_model`. # Test: Search for the `can_write_model` function implementation. rg --type cairo -A 10 $'fn can_write_model'Length of output: 74
Script:
#!/bin/bash # Description: Verify the function logic and integration of `can_write_model`. # Test: Search for the `can_write_model` function implementation in files with `.cairo` extension. rg --glob '*.cairo' -A 10 'fn can_write_model'Length of output: 1741
Script:
#!/bin/bash # Description: Verify the `_check_model_write_access` function and the `self.resources.read()` method, along with the `ResourceData` enum. # Test: Search for the `_check_model_write_access` function implementation. rg --glob '*.cairo' -A 10 '_check_model_write_access' # Test: Search for the `self.resources.read` method implementation. rg --glob '*.cairo' -A 10 'fn read' # Test: Search for the `ResourceData` enum definition. rg --glob '*.cairo' -A 10 'enum ResourceData'Length of output: 3510
Script:
#!/bin/bash # Description: Verify `is_writer`, `is_account_owner`, `is_account_writer`, `_check_basic_write_access`, and the `read` method. # Test: Search for the `is_writer` function implementation. rg --glob '*.cairo' -A 10 'fn is_writer' # Test: Search for the `is_account_owner` function implementation. rg --glob '*.cairo' -A 10 'fn is_account_owner' # Test: Search for the `is_account_writer` function implementation. rg --glob '*.cairo' -A 10 'fn is_account_writer' # Test: Search for the `_check_basic_write_access` function implementation. rg --glob '*.cairo' -A 10 '_check_basic_write_access' # Test: Search for the `read` method implementation. rg --glob '*.cairo' -A 10 'fn read'Length of output: 6887
examples/spawn-and-move/manifests/dev/deployment/manifest.json (10)
4-5: Class hash updates approved.The class hash and original class hash have been updated. Ensure that these hashes are correct and reflect the appropriate contract versions.
54-71: Addition ofModelIndexenum approved.The addition of the
ModelIndexenum with variantsKeys,Id, andMemberIdenhances the entity identification mechanism. This change aligns with the PR objectives to streamline entity handling.
308-313: Update toentityfunction parameters approved.The
entityfunction parameters have been updated to usemodel_selectorandindexinstead ofmodelandkeys. This change aligns with the newModelIndexenum and enhances clarity.
332-337: Update toset_entityfunction parameters approved.The
set_entityfunction parameters have been updated to usemodel_selectorandindexinstead ofmodelandkeys. This change aligns with the newModelIndexenum and enhances clarity.
356-361: Update todelete_entityfunction parameters approved.The
delete_entityfunction parameters have been updated to usemodel_selectorandindexinstead ofmodelandkeys. This change aligns with the newModelIndexenum and enhances clarity.
Line range hint
927-941:
Addition ofStoreUpdateRecordevent approved.The
StoreUpdateRecordevent has been added with membersentity_idandvalues. This change aligns with the PR objectives to enhance event structures and ensure consistency.
948-962: Addition ofStoreDelRecordevent approved.The
StoreDelRecordevent has been added with memberstableandentity_id. This change aligns with the PR objectives to enhance event structures and ensure consistency.
1156-1157: Update to address and transaction hash approved.The address and transaction hash have been updated. Ensure that these values are correct and reflect the appropriate deployment details.
1177-1179: Class hash updates approved.The class hash and original class hash have been updated. Ensure that these hashes are correct and reflect the appropriate contract versions.
1608-1610: Class hash updates approved.The class hash and original class hash have been updated. Ensure that these hashes are correct and reflect the appropriate contract versions.
crates/dojo-core/src/utils.cairo
Outdated
| /// TODO | ||
| fn find_model_field_layout( | ||
| model_layout: dojo::database::introspect::Layout, member_id: felt252 | ||
| ) -> Option<dojo::database::introspect::Layout> { | ||
| match model_layout { | ||
| dojo::database::introspect::Layout::Struct(struct_layout) => { | ||
| let mut i = 0; | ||
| let layout = loop { | ||
| if i >= struct_layout.len() { | ||
| break Option::None; | ||
| } | ||
|
|
||
| let field_layout = *struct_layout.at(i); | ||
| if field_layout.selector == member_id { | ||
| break Option::Some(field_layout.layout); | ||
| } | ||
| i += 1; | ||
| }; | ||
|
|
||
| layout | ||
| }, | ||
| _ => { | ||
| // should never happen as model layouts are always struct layouts. | ||
| panic_with_felt252('Unexpected model layout'); | ||
| Option::None | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
LGTM! But address the TODO comment.
The find_model_field_layout function is correct, but the TODO comment should be addressed.
Do you want me to address the TODO comment or open a GitHub issue to track this task?
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_struct_complex_array_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion was skipped due to low confidence.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| world.delete_entity(selector, keys, layout); | ||
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_delete_entity_with_complex_array_struct_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| world.delete_entity(selector, keys, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_struct_simple_array_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_nested_elements is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion was skipped due to low confidence.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_fixed_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | ||
|
|
||
| world.delete_entity(selector, keys, layout); | ||
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_delete_entity_with_fixed_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | |
| world.delete_entity(selector, keys, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(get_key_test()), values, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| world.delete_entity(selector, keys, layout); | ||
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | ||
|
|
||
| let expected_values = array![0, 0].span(); | ||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_delete_entity_with_struct_generics_enum_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| world.delete_entity(selector, keys, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_values = array![0, 0].span(); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| world.delete_entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); | |
| let expected_values = array![0, 0].span(); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_struct_layout_and_byte_array is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | ||
| assert_array(read_values, values); | ||
|
|
||
| // then override with the second variant | ||
| let values = create_struct_with_enum_second_variant(); | ||
| world.set_entity(selector, keys, values, layout); | ||
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | ||
|
|
||
| let read_values = world.entity(selector, keys, layout); | ||
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); |
There was a problem hiding this comment.
Add assertion for key retrieval
The function test_set_entity_with_struct_enum_layout is correct. Adding an assertion to check the correctness of the retrieved keys would improve the test coverage.
+ let expected_keys = get_key_test();
+ assert_array(keys, expected_keys, "Keys do not match the expected value.");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| assert_array(read_values, values); | |
| // then override with the second variant | |
| let values = create_struct_with_enum_second_variant(); | |
| world.set_entity(selector, keys, values, layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, keys, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| assert_array(read_values, values); | |
| // then override with the second variant | |
| let values = create_struct_with_enum_second_variant(); | |
| world.set_entity(selector, dojo::model::ModelIndex::Keys(keys), values, layout); | |
| let read_values = world.entity(selector, dojo::model::ModelIndex::Keys(keys), layout); | |
| let expected_keys = get_key_test(); | |
| assert_array(keys, expected_keys, "Keys do not match the expected value."); |
remybar
force-pushed
the
feat-get_by_entity_id
branch
2 times, most recently
from
6cb09f5 to
8871221
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (6)
crates/dojo-lang/src/semantics/test_data/set (1)
Test case for "set successful expansion" is missing the
Modelderive.The comprehensive search did not find the
Modelderive macro implementation in the expected format. This indicates that theModelderive is either not implemented correctly or missing. The test case expects aModelderive, which is necessary for the test to pass.
- File:
crates/dojo-lang/src/semantics/test_data/set- Lines: 119-139
- Issue: Missing
Modelderive macro implementation.Please implement the
Modelderive macro or correct the test case expectations.Analysis chain
Line range hint
23-147:
Test case for "set successful expansion" looks good.The expected output and diagnostics match the scenario of successfully expanding the
set!macro. However, there is an error about a missingModelderive.Verify the implementation of the
Modelderive to ensure it is correctly included.
Let's perform a more comprehensive search to confirm the presence or absence of the
Modelderive in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `Model` derive. # Test: Search for the `Model` derive implementation. Expect: At least one occurrence. rg --type rust 'derive(Model)'Length of output: 32
Script:
#!/bin/bash # Description: Verify the implementation of the `Model` derive more comprehensively. # Test 1: Search for the `Model` derive implementation in all Rust files. rg 'derive(Model)' # Test 2: Search for the definition of the `Model` trait. rg 'trait Model' # Test 3: Search for any references to a `Model` derive macro. rg 'Model'Length of output: 297826
crates/dojo-lang/src/inline_macros/delete.rs (1)
Line range hint
21-99:
Suggestion: Add error handling for potential issues during code generation.Consider adding error handling to manage potential issues during the code generation process.
+ if let Err(e) = dojo::model::Model::delete(@__delete_model_instance__, {}) { + // Handle error (e.g., log it, return from function, etc.) + }examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (3)
327-332: Update all instances ofset_entityto use the new parameters.The
set_entityfunction has instances with both the old and new signatures. Please ensure that all occurrences are updated to use the new parameters "model_selector" and "index".
bin/sozo/tests/test_data/manifest.jsoncrates/katana/storage/db/benches/artifacts/dojo_world_240.jsonAnalysis chain
LGTM! But verify the function usage in the codebase.
The changes to the
set_entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `set_entity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type json -A 5 $'"set_entity"'Length of output: 3875
303-308: Update Required: Old function signature still presentThe
entityfunction still uses the old parameter namemodelin some files. Please update these instances to use the new parametersmodel_selectorandindex:
crates/katana/storage/db/benches/artifacts/dojo_world_240.jsonbin/sozo/tests/test_data/manifest.jsonAnalysis chain
LGTM! But verify the function usage in the codebase.
The changes to the
entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `entity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type json -A 5 $'"entity"'Length of output: 3843
351-356: Issues Found: Updatedelete_entityFunction CallsThe
delete_entityfunction still uses the old parametermodelin some files. Please update these instances to use the new parametersmodel_selectorandindex.
crates/katana/storage/db/benches/artifacts/dojo_world_240.jsonbin/sozo/tests/test_data/manifest.jsonPlease ensure that all occurrences of
delete_entityacross the codebase are updated to match the new signature.Analysis chain
LGTM! But verify the function usage in the codebase.
The changes to the
delete_entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.
The initial search results indicate that some instances of
delete_entitystill use the old parametermodel. We need to verify that all instances have been updated to use the new parametersmodel_selectorandindex.Let's run a script to search for additional instances of
delete_entityand verify their parameters.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `delete_entity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type json -A 5 $'"delete_entity"'Length of output: 3899
Script:
#!/bin/bash # Additional verification to ensure no instances of `delete_entity` use old parameters. # Search for `delete_entity` occurrences and check their parameters. rg --type json -A 5 '"delete_entity"'Length of output: 3899
crates/dojo-world/src/contracts/abi/world.rs (1)
333-338: Update required: Ensure all calls toset_entitymatch the new signatureThe following instances of
set_entitycalls do not match the new signature and need to be updated to includemodel_selectorandindex:
crates/torii/libp2p/src/server/mod.rs
- Lines surrounding:
set_entity(ty, &message_id.to_string(), Utc::now().timestamp() as u64)crates/torii/graphql/src/tests/subscription_test.rs
- Lines surrounding:
db.set_entity(Ty::Struct(Struct { ... }))Please update these calls to the new signature using
model_selectorandindex.Analysis chain
LGTM! But verify the function usage in the codebase.
The changes to the
set_entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `set_entity` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type rust -A 5 $'set_entity'Length of output: 10405
Script:
#!/bin/bash # Description: Verify all function calls to `set_entity` match the new signature with correct arguments. # Test: Extract context around each call to `set_entity` to check for correct arguments. rg --type rust -A 10 -B 5 $'set_entity'Length of output: 25564
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (40)
- crates/dojo-core/src/base_test.cairo (1 hunks)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (25 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (8 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (5 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (8 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (2 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (16 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/dojo-core/src/base_test.cairo
Files skipped from review as they are similar to previous changes (31)
- crates/dojo-core/src/lib.cairo
- crates/dojo-core/src/model.cairo
- crates/dojo-core/src/model_test.cairo
- crates/dojo-core/src/resource_metadata.cairo
- crates/dojo-core/src/utils.cairo
- crates/dojo-core/src/world_test.cairo
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/inline_macros/set.rs
- crates/dojo-lang/src/introspect/layout.rs
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- crates/dojo-lang/src/model.rs
- crates/dojo-lang/src/semantics/test_data/get
- crates/dojo-world/src/contracts/model.rs
- crates/dojo-world/src/contracts/naming.rs
- crates/sozo/ops/src/events.rs
- crates/torii/types-test/src/contracts.cairo
- examples/spawn-and-move/Scarb.toml
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
- examples/spawn-and-move/src/actions.cairo
- examples/spawn-and-move/src/mock_token.cairo
Additional comments not posted (37)
crates/dojo-lang/src/semantics/test_data/set (2)
Line range hint
1-11:
Test case for "no params" looks good.The expected output and diagnostics match the scenario of providing no parameters to the
set!macro.
Line range hint
12-22:
Test case for "incorrect args" looks good.The expected output and diagnostics match the scenario of providing incorrect arguments to the
set!macro.crates/dojo-lang/src/inline_macros/delete.rs (3)
Line range hint
17-19:
StructDeleteMacrolooks good.The struct definition is correct and follows the pattern for a macro implementation.
Line range hint
21-99:
Functiongenerate_codelooks good.The implementation correctly handles the
deletemacro logic, including argument validation and code generation.
Line range hint
100-166:
Result construction looks good.The construction of the
InlinePluginResultwith generated code and diagnostics follows the expected pattern.examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (3)
277-288: Functionupdate_player_namelooks good.The function definition appears correct and follows the expected pattern with the appropriate inputs and state mutability.
289-300: Functionupdate_player_name_valuelooks good.The function definition appears correct and follows the expected pattern with the appropriate inputs and state mutability.
324-334: Functioncall_somethinglooks good.The function definition appears correct and follows the expected pattern with the appropriate inputs and state mutability.
examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json (3)
1-6: StructDojoModelImpllooks good.The struct definition appears correct and follows the expected pattern with the appropriate interface implementation.
7-24: Structcore::byte_array::ByteArraylooks good.The struct definition appears correct and follows the expected pattern with the appropriate members.
25-38: Enumcore::option::Option::<core::integer::u32>looks good.The enum definition appears correct and follows the expected pattern with the appropriate variants.
examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (2)
49-66: LGTM! Addition ofdojo::world::ModelIndexenum.The enum
dojo::world::ModelIndexis well-defined with three variants:Keys,Id, andMemberId, each having appropriate associated types.
506-506: LGTM! But verify the function usage in the codebase.The changes to the
can_write_modelfunction to usemodel_selectorare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.Verification successful
Verification successful: The
can_write_modelfunction is consistently usingmodel_selectoracross the codebase.The changes to the function signature have been correctly propagated throughout the relevant files.
examples/spawn-and-move/manifests/dev/deployment/manifest.jsonexamples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.jsonexamples/spawn-and-move/manifests/release/base/abis/dojo-world.jsonexamples/spawn-and-move/manifests/dev/base/abis/dojo-world.jsoncrates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.jsonScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `can_write_model` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type json -A 5 $'"can_write_model"'Length of output: 3007
crates/dojo-world/src/contracts/abi/world.rs (4)
55-72: LGTM! Addition ofdojo::world::ModelIndexenum.The enum
dojo::world::ModelIndexis well-defined with three variants:Keys,Id, andMemberId, each having appropriate associated types.
357-362: LGTM! But verify the function usage in the codebase.The changes to the
delete_entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.
512-512: LGTM! But verify the function usage in the codebase.The changes to the
can_write_modelfunction to usemodel_selectorare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.
309-314: LGTM! But verify the function usage in the codebase.The changes to the
entityfunction to usemodel_selectorandindexare approved. Ensure that all calls to this function in the codebase are updated to match the new signature.crates/dojo-core/src/world.cairo (10)
6-12: Introduction ofModelIndexenum is a good addition.The
ModelIndexenum introduces a structured way to handle different types of indices, which can be useful for various operations on models.
224-229: Introduction ofStoreUpdateRecordandStoreDelRecordstructs is beneficial.These structs provide a structured way to handle events related to model updates and deletions.
Also applies to: 234-234
751-781: Updates in theentitymethod improve flexibility.The use of the
ModelIndexenum allows theentitymethod to handle different types of indices, enhancing its flexibility.
793-822: Updates in theset_entitymethod improve flexibility.The use of the
ModelIndexenum allows theset_entitymethod to handle different types of indices, enhancing its flexibility.
835-859: Updates in thedelete_entitymethod improve flexibility.The use of the
ModelIndexenum allows thedelete_entitymethod to handle different types of indices, enhancing its flexibility.
Line range hint
1016-1031:
Enhancements in_check_model_write_accessimprove security.The additional checks in the
_check_model_write_accessmethod enhance the security and robustness of the access control mechanism.
1066-1084: Updates in_write_model_entitymethod improve flexibility.The use of the
Layoutenum allows the_write_model_entitymethod to handle different layout types, enhancing its flexibility and robustness.
1096-1107: Updates in_delete_model_entitymethod improve flexibility.The use of the
Layoutenum allows the_delete_model_entitymethod to handle different layout types, enhancing its flexibility and robustness.
1119-1132: Updates in_read_model_entitymethod improve flexibility.The use of the
Layoutenum allows the_read_model_entitymethod to handle different layout types, enhancing its flexibility and robustness.
27-32: Updates inIWorld<T>trait improve flexibility and robustness.The introduction of new methods and the use of
ModelIndexenhance the flexibility and robustness of model operations.Ensure that these updated methods are used correctly throughout the codebase.
Also applies to: 36-37, 42-45, 62-62
Verification successful
The updated methods in
IWorld<T>trait are correctly used throughout the codebase.The methods
entity,set_entity,delete_entity, andcan_write_modelare consistently used in various files, indicating proper integration of the changes.
- Verified usage in:
crates/torii/libp2p/src/server/mod.rs,crates/torii/core/src/sql.rs,crates/dojo-world/src/contracts/model.rs,crates/dojo-lang/src/model.rs,crates/dojo-core/src/world.cairo, and other related files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of updated methods in `IWorld<T>` trait. # Test: Search for the usage of `entity`, `set_entity`, `delete_entity`, and `can_write_model` methods. rg --type cairo -A 5 $'entity\(|set_entity\(|delete_entity\(|can_write_model\('Length of output: 110
Script:
#!/bin/bash # Description: Verify the usage of updated methods in `IWorld<T>` trait. # Test: Search for the usage of `entity`, `set_entity`, `delete_entity`, and `can_write_model` methods in all files (no specific type). rg -A 5 $'entity\(|set_entity\(|delete_entity\(|can_write_model\('Length of output: 156600
examples/spawn-and-move/manifests/dev/deployment/manifest.json (10)
4-5: Verify the correctness of the updated hash values.Ensure that the new
class_hashandoriginal_class_hashvalues forWorldContractare correct and consistent with the expected values.
54-71: LGTM! Verify the usage of the newModelIndexenum.The
ModelIndexenum with variantsKeys,Id, andMemberIdlooks good.Ensure that the usage of this enum in the codebase is correct and consistent.
308-313: LGTM! Verify the usage of updated function parameters.The updated input parameters
model_selectorandindexfor the functionentityare more descriptive and align with theModelIndexenum.Ensure that the changes are correctly reflected throughout the codebase.
332-337: LGTM! Verify the usage of updated function parameters.The updated input parameters
model_selectorandindexfor the functionset_entityare more descriptive and align with theModelIndexenum.Ensure that the changes are correctly reflected throughout the codebase.
356-361: LGTM! Verify the usage of updated function parameters.The updated input parameters
model_selectorandindexfor the functiondelete_entityare more descriptive and align with theModelIndexenum.Ensure that the changes are correctly reflected throughout the codebase.
511-511: LGTM! Verify the usage of updated function parameter.The updated input parameter
model_selectorfor the functioncan_write_modelis more descriptive and aligns with theModelIndexenum.Ensure that the change is correctly reflected throughout the codebase.
Line range hint
927-941:
LGTM! Verify the usage of renamed event and modified members.The event
StoreDelRecordhas been renamed toStoreUpdateRecordwith modified members. This reflects the new functionality for updating records.Ensure that the changes are correctly reflected throughout the codebase.
948-962: LGTM! Verify the usage of the new eventStoreDelRecord.The new event
StoreDelRecordwith memberstableandentity_idlooks good.Ensure that the usage of this event in the codebase is correct and consistent.
1156-1157: Verify the correctness of the updated address and transaction hash values.Ensure that the new values for
addressandtransaction_hashin theworldsection are correct and consistent with the expected values.
1177-1179: Verify the correctness of the updated hash values.Ensure that the new
class_hashandoriginal_class_hashvalues forDojoContractare correct and consistent with the expected values.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
crates/dojo-core/src/utils.cairo (1)
8-19: Ensure proper error handling forentity_id_from_keys.The function
entity_id_from_keyscorrectly usesposeidon_hash_span. However, consider adding error handling in case the hash computation fails.fn entity_id_from_keys(keys: Span<felt252>) -> felt252 { - poseidon::poseidon_hash_span(keys) + match poseidon::poseidon_hash_span(keys) { + Ok(hash) => hash, + Err(e) => { + // Handle the error appropriately + panic!("Failed to compute hash: {:?}", e); + } + } }crates/dojo-core/src/model.cairo (3)
Line range hint
52-74: Consider adding detailed documentation fordeploy_and_get_metadata.The function
deploy_and_get_metadatashould have detailed documentation explaining the parameters and return values./// Deploys a model with the given [`ClassHash`] and retrieves its name. /// Currently, the model is expected to already be declared by `sozo`. /// /// # Arguments /// /// * `salt` - A salt used to uniquely deploy the model. /// * `class_hash` - Class Hash of the model. /// /// # Returns /// /// * `SyscallResult<(starknet::ContractAddress, ByteArray, felt252, ByteArray, felt252)>` - A tuple containing: /// * `contract_address` - The address of the deployed contract. /// * `name` - The name of the model. /// * `selector` - The selector of the model. /// * `namespace` - The namespace of the model. /// * `namespace_hash` - The hash of the namespace. fn deploy_and_get_metadata( salt: felt252, class_hash: starknet::ClassHash ) -> SyscallResult<(starknet::ContractAddress, ByteArray, felt252, ByteArray, felt252)> { let (contract_address, _) = starknet::deploy_syscall( class_hash, salt, array![].span(), false, )?; let model = IModelDispatcher { contract_address }; let name = model.name(); let selector = model.selector(); let namespace = model.namespace(); let namespace_hash = model.namespace_hash(); Result::Ok((contract_address, name, selector, namespace, namespace_hash)) }
Ensure
Modelmethods handle errors gracefully.The
Modeltrait methods do not currently handle errors gracefully, as there are no patterns indicating error handling practices such asResult,Error,panic,unwrap, orexpect.
- Lines: 1-32 in
crates/dojo-core/src/model.cairoAnalysis chain
Line range hint
17-50: EnsureModelmethods handle errors gracefully.The methods in the
Modeltrait should handle potential errors gracefully, especially when interacting with theworld.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure `Model` methods handle errors gracefully. # Test: Check for proper error handling in `Model` methods. rg --type cairo -A 5 $'trait Model' | grep -i "Result" > /dev/null || echo "No error handling found in Model methods."Length of output: 176
Script:
#!/bin/bash # Searching for the definition of the `Model` trait and its methods to check for error handling. rg -A 20 'trait Model' --glob '*.cairo' | grep -i -E 'Result|Error|panic|unwrap|expect' > /dev/null || echo "No error handling found in Model methods."Length of output: 182
5-16: EnsureModelEntitymethods handle errors gracefully.The methods in the
ModelEntitytrait do not currently handle errors gracefully, as there is no usage ofResultor similar error handling mechanisms.
- File:
crates/dojo-core/src/model.cairo
- Lines: 5-16
Analysis chain
Ensure
ModelEntitymethods handle errors gracefully.The methods in the
ModelEntitytrait should handle potential errors gracefully, especially when interacting with theworld.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure `ModelEntity` methods handle errors gracefully. # Test: Check for proper error handling in `ModelEntity` methods. rg --type cairo -A 5 $'trait ModelEntity' | grep -i "Result" > /dev/null || echo "No error handling found in ModelEntity methods."Length of output: 194
Script:
#!/bin/bash # Description: Ensure `ModelEntity` methods handle errors gracefully. # Find the file containing the trait `ModelEntity` fd --type f --exec rg -l 'trait ModelEntity' > model_entity_files.txt # Check for proper error handling in `ModelEntity` methods while IFS= read -r file; do echo "Checking $file for error handling in ModelEntity methods..." rg -A 5 'trait ModelEntity' "$file" | grep -i "Result" > /dev/null || echo "No error handling found in ModelEntity methods in $file." done < model_entity_files.txtLength of output: 540
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (40)
- crates/dojo-core/src/base_test.cairo (1 hunks)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (25 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (8 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (5 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (8 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (2 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (16 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/dojo-core/src/base_test.cairo
Files skipped from review as they are similar to previous changes (30)
- crates/dojo-core/src/lib.cairo
- crates/dojo-core/src/model_test.cairo
- crates/dojo-core/src/resource_metadata.cairo
- crates/dojo-core/src/world_test.cairo
- crates/dojo-lang/src/inline_macros/delete.rs
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/introspect/layout.rs
- crates/dojo-lang/src/model.rs
- crates/dojo-lang/src/semantics/test_data/get
- crates/dojo-lang/src/semantics/test_data/set
- crates/dojo-world/src/contracts/abi/world.rs
- crates/dojo-world/src/contracts/model.rs
- crates/dojo-world/src/contracts/naming.rs
- crates/sozo/ops/src/events.rs
- crates/torii/types-test/src/contracts.cairo
- examples/spawn-and-move/Scarb.toml
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
- examples/spawn-and-move/src/actions.cairo
- examples/spawn-and-move/src/mock_token.cairo
Additional comments not posted (28)
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1)
2-3: Validateclass_hashandoriginal_class_hash.Ensure that the
class_hashandoriginal_class_hashvalues are correct and consistent with the expected values for thedojo-worldclass.Verification successful
The
class_hashandoriginal_class_hashvalues are correct.The verification script confirms that both
class_hashandoriginal_class_hashmatch the expected value0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e.
class_hash:0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7eoriginal_class_hash:0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7eScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Validate `class_hash` and `original_class_hash` values. # Test: Check for the expected values of `class_hash` and `original_class_hash`. expected_class_hash="0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e" actual_class_hash=$(rg --no-heading --line-number --color never 'class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | awk '{print $3}') if [ "$actual_class_hash" != "$expected_class_hash" ]; then echo "class_hash value is incorrect." exit 1 fi actual_original_class_hash=$(rg --no-heading --line-number --color never 'original_class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | awk '{print $3}') if [ "$actual_original_class_hash" != "$expected_class_hash" ]; then echo "original_class_hash value is incorrect." exit 1 fi echo "class_hash and original_class_hash values are correct."Length of output: 712
Script:
#!/bin/bash # Description: Validate `class_hash` and `original_class_hash` values. # Test: Check for the expected values of `class_hash` and `original_class_hash`. expected_class_hash="0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e" # Extract actual values without quotes actual_class_hash=$(rg --no-heading --line-number --color never 'class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | awk -F' = ' '{print $2}' | tr -d '"') actual_original_class_hash=$(rg --no-heading --line-number --color never 'original_class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | awk -F' = ' '{print $2}' | tr -d '"') # Check if the values are correct if [ "$actual_class_hash" != "$expected_class_hash" ]; then echo "class_hash value is incorrect." exit 1 fi if [ "$actual_original_class_hash" != "$expected_class_hash" ]; then echo "original_class_hash value is incorrect." exit 1 fi echo "class_hash and original_class_hash values are correct."Length of output: 1018
Script:
#!/bin/bash # Description: Validate `class_hash` and `original_class_hash` values. # Test: Check for the expected values of `class_hash` and `original_class_hash`. expected_class_hash="0x2bcf11feac030c5ec81496dfc73ae256568c5b7518f8542e25dd0e42938de7e" # Extract actual values without quotes and only the first occurrence actual_class_hash=$(rg --no-heading --line-number --color never 'class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | head -n 1 | awk -F' = ' '{print $2}' | tr -d '"') actual_original_class_hash=$(rg --no-heading --line-number --color never 'original_class_hash' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml | head -n 1 | awk -F' = ' '{print $2}' | tr -d '"') # Check if the values are correct if [ "$actual_class_hash" != "$expected_class_hash" ]; then echo "class_hash value is incorrect." exit 1 fi if [ "$actual_original_class_hash" != "$expected_class_hash" ]; then echo "original_class_hash value is incorrect." exit 1 fi echo "class_hash and original_class_hash values are correct."Length of output: 1094
crates/dojo-lang/src/inline_macros/set.rs (1)
163-164: Ensure correct handling of model instances inSetMacro.The code correctly sets the model instance, but ensure that the model instances are valid and properly handled.
examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (4)
277-288: LGTM!The addition of the
update_player_namefunction looks good. Ensure to add corresponding tests.
289-300: LGTM!The addition of the
update_player_name_valuefunction looks good. Ensure to add corresponding tests.
Line range hint
922-936:
LGTM!The addition of the
StoreUpdateRecordevent looks good.
Line range hint
943-956:
LGTM!The addition of the
StoreDelRecordevent looks good.examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json (2)
1-6: LGTM!The addition of the
DojoModelImplimplementation for thedojo::model::IModelinterface looks good.
365-368: LGTM!The addition of the
directions_availableImplimplementation for thedojo_examples::models::Idirections_availableinterface looks good.crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (6)
49-65: LGTM!The addition of the
ModelIndexenum looks good.
303-308: LGTM!The addition of the
model_selectorandindexinputs to theentityfunction looks good.
327-332: LGTM!The addition of the
model_selectorandindexinputs to theset_entityfunction looks good.
351-356: LGTM!The addition of the
model_selectorandindexinputs to thedelete_entityfunction looks good.
Line range hint
506-510:
LGTM!The addition of the
can_write_modelfunction looks good.
1118-1122: LGTM!The addition of the
StoreUpdateRecordevent looks good.examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (4)
49-65: LGTM! New enumModelIndexThe new
ModelIndexenum with variantsKeys,Id, andMemberIdis a well-structured addition for model indexing.
303-308: LGTM! Changes toentityfunction inputsThe introduction of
model_selectorandindexparameters in theentityfunction aligns with the newModelIndexenum, enhancing model selection and indexing capabilities.
327-332: LGTM! Changes toset_entityfunction inputsThe introduction of
model_selectorandindexparameters in theset_entityfunction aligns with the newModelIndexenum, enhancing model selection and indexing capabilities.
351-356: LGTM! Changes todelete_entityfunction inputsThe introduction of
model_selectorandindexparameters in thedelete_entityfunction aligns with the newModelIndexenum, enhancing model selection and indexing capabilities.crates/dojo-core/src/world.cairo (10)
6-12: EnumModelIndexlooks good!The
ModelIndexenum is defined correctly with three variants:Keys,Id, andMemberId.
27-32: TraitIWorldupdates look good!The new methods
entity,set_entity,delete_entity,can_write_model, andcan_write_namespaceprovide additional functionality for managing models and namespaces.Also applies to: 36-37, 42-45, 62-64
156-157: EnumEventupdates look good!The new event variants
StoreSetRecord,StoreUpdateRecord, andStoreDelRecordare defined correctly.Also applies to: 224-230, 234-235
317-322: Functionmetadataupdates look good!The usage of
entity_id_from_keysfor generating the entity ID from the model keys is appropriate.
345-349: Functionset_metadataupdates look good!The usage of
Model::entity_id,Model::values, andModel::layoutfor setting the metadata values is appropriate.
503-516: Functioncan_write_modellooks good!The function correctly checks if a contract can write to a model by verifying the resource type and write access.
Line range hint
562-573:
Functioncan_write_namespacelooks good!The function correctly checks if a contract can write to a namespace by verifying the resource type and write access.
751-781: Functionentitylooks good!The function correctly uses the
ModelIndexenum to determine how to read the model data, which is a flexible approach.
793-822: Functionset_entitylooks good!The function correctly uses the
ModelIndexenum to determine how to write the model data, and it includes an appropriate assertion for write access.
835-859: Functiondelete_entitylooks good!The function correctly uses the
ModelIndexenum to determine how to delete the model data, and it includes an appropriate assertion for write access.
| /// Find the layout of a model field based on its selector. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `model_layout` - The full model layout (must be a Layout::Struct). | ||
| /// * `member_selector` - The model field selector. | ||
| /// | ||
| /// # Returns | ||
| /// Some(Layout) if the field layout has been found, None otherwise. | ||
| fn find_model_field_layout( | ||
| model_layout: dojo::database::introspect::Layout, member_selector: felt252 | ||
| ) -> Option<dojo::database::introspect::Layout> { | ||
| match model_layout { | ||
| dojo::database::introspect::Layout::Struct(struct_layout) => { | ||
| let mut i = 0; | ||
| let layout = loop { | ||
| if i >= struct_layout.len() { | ||
| break Option::None; | ||
| } | ||
|
|
||
| let field_layout = *struct_layout.at(i); | ||
| if field_layout.selector == member_selector { | ||
| break Option::Some(field_layout.layout); | ||
| } | ||
| i += 1; | ||
| }; | ||
|
|
||
| layout | ||
| }, | ||
| _ => { | ||
| // should never happen as model layouts are always struct layouts. | ||
| panic_with_felt252('Unexpected model layout'); | ||
| Option::None | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider refactoring find_model_field_layout for clarity.
The function find_model_field_layout is correct but could be refactored for better readability and maintainability.
fn find_model_field_layout(
model_layout: dojo::database::introspect::Layout, member_selector: felt252
) -> Option<dojo::database::introspect::Layout> {
match model_layout {
dojo::database::introspect::Layout::Struct(struct_layout) => {
struct_layout.iter().find_map(|field_layout| {
if field_layout.selector == member_selector {
Some(field_layout.layout)
} else {
None
}
})
},
_ => {
// should never happen as model layouts are always struct layouts.
panic_with_felt252('Unexpected model layout');
Option::None
}
}
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Find the layout of a model field based on its selector. | |
| /// | |
| /// # Arguments | |
| /// | |
| /// * `model_layout` - The full model layout (must be a Layout::Struct). | |
| /// * `member_selector` - The model field selector. | |
| /// | |
| /// # Returns | |
| /// Some(Layout) if the field layout has been found, None otherwise. | |
| fn find_model_field_layout( | |
| model_layout: dojo::database::introspect::Layout, member_selector: felt252 | |
| ) -> Option<dojo::database::introspect::Layout> { | |
| match model_layout { | |
| dojo::database::introspect::Layout::Struct(struct_layout) => { | |
| let mut i = 0; | |
| let layout = loop { | |
| if i >= struct_layout.len() { | |
| break Option::None; | |
| } | |
| let field_layout = *struct_layout.at(i); | |
| if field_layout.selector == member_selector { | |
| break Option::Some(field_layout.layout); | |
| } | |
| i += 1; | |
| }; | |
| layout | |
| }, | |
| _ => { | |
| // should never happen as model layouts are always struct layouts. | |
| panic_with_felt252('Unexpected model layout'); | |
| Option::None | |
| } | |
| } | |
| } | |
| fn find_model_field_layout( | |
| model_layout: dojo::database::introspect::Layout, member_selector: felt252 | |
| ) -> Option<dojo::database::introspect::Layout> { | |
| match model_layout { | |
| dojo::database::introspect::Layout::Struct(struct_layout) => { | |
| struct_layout.iter().find_map(|field_layout| { | |
| if field_layout.selector == member_selector { | |
| Some(field_layout.layout) | |
| } else { | |
| None | |
| } | |
| }) | |
| }, | |
| _ => { | |
| // should never happen as model layouts are always struct layouts. | |
| panic_with_felt252('Unexpected model layout'); | |
| Option::None | |
| } | |
| } | |
| } |
remybar
force-pushed
the
feat-get_by_entity_id
branch
from
8871221 to
946b627
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (40)
- crates/dojo-core/src/base_test.cairo (1 hunks)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (25 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (8 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (5 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (8 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (2 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (16 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (8 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/dojo-core/src/lib.cairo
Files skipped from review as they are similar to previous changes (34)
- crates/dojo-core/src/base_test.cairo
- crates/dojo-core/src/model.cairo
- crates/dojo-core/src/model_test.cairo
- crates/dojo-core/src/resource_metadata.cairo
- crates/dojo-core/src/utils.cairo
- crates/dojo-core/src/world.cairo
- crates/dojo-core/src/world_test.cairo
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/inline_macros/set.rs
- crates/dojo-lang/src/introspect/layout.rs
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- crates/dojo-lang/src/model.rs
- crates/dojo-lang/src/semantics/test_data/set
- crates/dojo-world/src/contracts/abi/world.rs
- crates/dojo-world/src/contracts/model.rs
- crates/dojo-world/src/contracts/naming.rs
- crates/sozo/ops/src/events.rs
- crates/torii/types-test/src/contracts.cairo
- examples/spawn-and-move/Scarb.toml
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/deployment/abis/models/dojo_examples-DirectionsAvailable-22b3dd08.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
- examples/spawn-and-move/src/mock_token.cairo
Additional comments not posted (17)
examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1)
2-3: LGTM! Theclass_hashandoriginal_class_hashvalues have been updated correctly.The changes to the
class_hashandoriginal_class_hashvalues appear to be correct.crates/dojo-lang/src/semantics/test_data/get (1)
204-204: LGTM! The test data for thegetmacro has been updated correctly.The changes to the test data for the
getmacro appear to be correct.examples/spawn-and-move/src/actions.cairo (6)
11-12: LGTM! The newupdate_player_nameandupdate_player_name_valuemethods have been added correctly.The new methods
update_player_nameandupdate_player_name_valuehave been added to theIActionstrait.
32-33: LGTM! The imports for the new traits have been added correctly.The imports for the new traits
PositionTrait,MovesTrait,MovesEntityTrait,PlayerConfigTrait, andPlayerConfigEntityTraithave been added correctly.
85-106: LGTM! The new methods for getting and setting model values have been added correctly.The new methods for getting and setting model values using the
<ModelName>Traitand<ModelName>EntityTraittraits have been added correctly.
125-129: LGTM! The new methods for deleting model values have been added correctly.The new methods for deleting model values using the
<ModelName>Traitand<ModelName>EntityTraittraits have been added correctly.
161-180: LGTM! The new methods for updating player names have been added correctly.The new methods for updating player names using the
<ModelName>Traitand<ModelName>EntityTraittraits have been added correctly.
Line range hint
233-249:
LGTM! The test for themovemethod has been updated correctly.The test for the
movemethod has been updated to use the newgetmacro correctly.examples/spawn-and-move/manifests/dev/deployment/manifest.json (9)
4-5: Verify the correctness of updated class hashes.The
class_hashandoriginal_class_hashfor the "world" object have been updated. Ensure these values are correct and consistent with the rest of the configuration.
54-71: Verify the correctness of the new enum definition.A new enum
ModelIndexhas been added with variantsKeys,Id, andMemberId. Ensure these definitions are correct and consistent with the rest of the configuration.
308-313: Verify the correctness of the updated function signature.The
entityfunction has been updated to include a newindexparameter of typeModelIndex. Ensure this update is correct and consistent with the rest of the configuration.
332-337: Verify the correctness of the updated function signature.The
set_entityfunction has been updated to include a newindexparameter of typeModelIndex. Ensure this update is correct and consistent with the rest of the configuration.
356-361: Verify the correctness of the updated function signature.The
delete_entityfunction has been updated to include a newindexparameter of typeModelIndex. Ensure this update is correct and consistent with the rest of the configuration.
511-511: Verify the correctness of the new function definition.The
can_write_modelfunction has been added with amodel_selectorparameter. Ensure this function is correctly defined and consistent with the rest of the configuration.
Line range hint
927-941:
Verify the correctness of the new event definition.A new event
StoreUpdateRecordhas been added with memberstable,entity_id, andvalues. Ensure this event is correctly defined and consistent with the rest of the configuration.
947-962: Verify the correctness of the new event definition.A new event
StoreDelRecordhas been added with memberstableandentity_id. Ensure this event is correctly defined and consistent with the rest of the configuration.
1156-1157: Verify the correctness of updated address and transaction hash.The
addressandtransaction_hashfor the "world" object have been updated. Ensure these values are correct and consistent with the rest of the configuration.
remybar
force-pushed
the
feat-get_by_entity_id
branch
from
946b627 to
b511241
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (3)
examples/spawn-and-move/src/actions.cairo (3)
170-188: Missing Implementation ofset_nameMethodsThe
set_namemethods for bothPlayerConfigTraitandPlayerConfigEntityTraitare not found in the codebase. This is critical as the provided code snippet relies on these methods.
- Ensure that
PlayerConfigTrait::set_nameandPlayerConfigEntityTrait::set_nameare implemented correctly.Analysis chain
Ensure consistency and correctness of the new trait methods.
The new trait methods for
PlayerConfigTraitandPlayerConfigEntityTraitare used to get and set entity values. Verify that these methods are correctly implemented and consistent with the existing code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Test: Search for the new trait methods in the codebase. Expect: No discrepancies. rg --type cairo 'PlayerConfigTrait::get' rg --type cairo 'PlayerConfigTrait::set_name' rg --type cairo 'PlayerConfigTrait::get_name' rg --type cairo 'PlayerConfigEntityTrait::get' rg --type cairo 'PlayerConfigEntityTrait::set_name' rg --type cairo 'PlayerConfigEntityTrait::get_name'Length of output: 463
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Test: Search for the new trait methods in the codebase. Expect: No discrepancies. rg 'PlayerConfigTrait::get' rg 'PlayerConfigTrait::set_name' rg 'PlayerConfigTrait::get_name' rg 'PlayerConfigEntityTrait::get' rg 'PlayerConfigEntityTrait::set_name' rg 'PlayerConfigEntityTrait::get_name'Length of output: 1002
Script:
#!/bin/bash # Description: Verify the implementation of the set_name methods. # Test: Search for the implementation of the set_name methods in the codebase. ast-grep --lang cairo --pattern 'fn set_name($_, $_) { $$$ }'Length of output: 174
Script:
#!/bin/bash # Description: Verify the implementation of the set_name methods. # Test: Search for the implementation of the set_name methods in the codebase. rg 'fn set_name'Length of output: 18
90-111: Issue withMovesEntityTrait::updatemethod implementationThe
MovesEntityTrait::updatemethod mentioned in the code snippet does not appear to be implemented in the codebase. This is a critical issue as it affects the consistency and correctness of the new trait methods.
MovesEntityTrait::updateis not defined or implemented in the codebase.Please ensure that the
MovesEntityTrait::updatemethod is correctly implemented and consistent with the existing code.Analysis chain
Ensure consistency and correctness of the new trait methods.
The new trait methods for
PositionTrait,MovesTrait, andMovesEntityTraitare used to get and set entity values. Verify that these methods are correctly implemented and consistent with the existing code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Test: Search for the new trait methods in the codebase. Expect: No discrepancies. rg --type cairo 'PositionTrait::get' rg --type cairo 'MovesTrait::entity_id_from_keys' rg --type cairo 'MovesEntityTrait::get' rg --type cairo 'MovesEntityTrait::update'Length of output: 289
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Find all Cairo files in the repository fd -e cairo > cairo_files.txt # Search for the new trait methods in the Cairo files xargs -a cairo_files.txt rg 'PositionTrait::get' xargs -a cairo_files.txt rg 'MovesTrait::entity_id_from_keys' xargs -a cairo_files.txt rg 'MovesEntityTrait::get' xargs -a cairo_files.txt rg 'MovesEntityTrait::update'Length of output: 557
Script:
#!/bin/bash # Description: Verify the implementation of the MovesEntityTrait and check for the update method. # Search for the implementation of MovesEntityTrait in the Cairo files fd -e cairo | xargs rg 'trait MovesEntityTrait' # Search for the definition of the update method in the Cairo files fd -e cairo | xargs rg 'fn update'Length of output: 931
Script:
#!/bin/bash # Description: Verify the definition and methods of MovesEntityTrait. # Search for the definition of MovesEntityTrait in the Cairo files fd -e cairo | xargs rg 'MovesEntityTrait' # Search for methods within MovesEntityTrait fd -e cairo | xargs rg 'impl MovesEntityTrait'Length of output: 322
130-134: Missing Definitions and Implementations forPlayerConfigTraitThe
PlayerConfigTraitis referenced multiple times in theexamples/spawn-and-move/src/actions.cairofile, but its definition and implementation are missing from the codebase. This means the trait methods (getanddelete) being used are not defined anywhere, which is a critical issue.
examples/spawn-and-move/src/actions.cairo:
- Lines referencing
PlayerConfigTrait::get- Lines referencing
PlayerConfigTrait::deletePlease ensure that the
PlayerConfigTraitand its methods are correctly defined and implemented.Analysis chain
Ensure consistency and correctness of the new trait methods.
The new trait methods for
PlayerConfigTraitare used to get and delete entity values. Verify that these methods are correctly implemented and consistent with the existing code.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Test: Search for the new trait methods in the codebase. Expect: No discrepancies. rg --type cairo 'PlayerConfigTrait::get' rg --type cairo 'PlayerConfigTrait::delete'Length of output: 144
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods. # Test: Search for the new trait methods in the codebase without specifying the file type. rg 'PlayerConfigTrait::get' rg 'PlayerConfigTrait::delete'Length of output: 383
Script:
#!/bin/bash # Description: Verify the implementation of the new trait methods. # Test: Search for the implementation of the new trait methods in the codebase. rg 'trait PlayerConfigTrait' rg 'fn get' rg 'fn delete'Length of output: 18284
Script:
#!/bin/bash # Description: Verify the implementation of the PlayerConfigTrait methods. # Test: Search for the definition of the PlayerConfigTrait and its methods specifically. rg 'trait PlayerConfigTrait' rg 'impl PlayerConfigTrait'Length of output: 60
Script:
#!/bin/bash # Description: Verify the existence and references of PlayerConfigTrait in the codebase. # Test: Search for any references to PlayerConfigTrait in the codebase. rg 'PlayerConfigTrait'Length of output: 588
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (37)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (21 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (7 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (6 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (7 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (14 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/dojo-core/src/lib.cairo
Files skipped from review as they are similar to previous changes (29)
- crates/dojo-core/src/model.cairo
- crates/dojo-core/src/resource_metadata.cairo
- crates/dojo-core/src/utils.cairo
- crates/dojo-core/src/world.cairo
- crates/dojo-core/src/world_test.cairo
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/introspect/layout.rs
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- crates/dojo-lang/src/model.rs
- crates/dojo-lang/src/semantics/test_data/get
- crates/dojo-lang/src/semantics/test_data/set
- crates/dojo-world/src/contracts/abi/world.rs
- crates/dojo-world/src/contracts/model.rs
- crates/dojo-world/src/contracts/naming.rs
- crates/sozo/ops/src/events.rs
- crates/torii/types-test/src/contracts.cairo
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.json
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
- examples/spawn-and-move/src/mock_token.cairo
Additional comments not posted (24)
examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1)
2-3: Verify the updated class hashes.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and correspond to the updateddojo-worldclass.crates/dojo-core/src/model_test.cairo (12)
5-7: LGTM!The
deploy_worldfunction correctly spawns a test world usingspawn_test_world.
21-24: LGTM!The
test_idfunction correctly tests theidmethod of theFooEntitystruct.
27-33: LGTM!The
test_valuesfunction correctly tests thevaluesmethod of theFooEntitystruct.
36-41: LGTM!The
test_from_valuesfunction correctly tests thefrom_valuesmethod of theFooEntitystruct.
44-48: LGTM!The
test_from_values_bad_datafunction correctly tests thefrom_valuesmethod of theFooEntitystruct with bad data and expects a panic.
51-69: LGTM!The
test_get_and_update_entityfunction correctly tests thegetandupdatemethods of theFooEntityTrait.
72-85: LGTM!The
test_delete_entityfunction correctly tests thedeletemethod of theFooEntityTrait.
88-107: LGTM!The
test_get_and_set_member_from_entityfunction correctly tests theget_memberandset_membermethods of theFooEntityTrait.
110-124: LGTM!The
test_get_and_set_field_namefunction correctly tests theget_v1andset_v1methods of theFooEntityTrait.
128-142: LGTM!The
test_get_and_set_from_modelfunction correctly tests thegetandsetmethods of theFooTrait.
146-160: LGTM!The
test_delete_from_modelfunction correctly tests thedeletemethod of theFooTrait.
164-180: LGTM!The
test_get_and_set_member_from_modelfunction correctly tests theget_memberandset_membermethods of theFooTrait.examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5)
71-71: Verify the correctness of the updatedclass_hashandoriginal_class_hashvalues.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the intended updates.
57-59: Verify the correctness of the updatedclass_hashandoriginal_class_hashvalues.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the intended updates.
3-4: Verify the correctness of the updatedclass_hashandoriginal_class_hashvalues.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the intended updates.
26-28: Verify the correctness of the updatedclass_hashandoriginal_class_hashvalues.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the intended updates.
43-43: Verify the correctness of the updatedclass_hashandoriginal_class_hashvalues.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the intended updates.examples/spawn-and-move/src/actions.cairo (3)
9-10: Verify the correctness of the new methods.Ensure that the new methods
update_player_nameandupdate_player_name_valueare correctly implemented and consistent with the existing code.Verification successful
The new methods
update_player_nameandupdate_player_name_valueare correctly implemented and consistent with the existing code.
- The methods retrieve the player's configuration.
- They set the new name and validate the update by asserting the new name matches the expected value.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new methods `update_player_name` and `update_player_name_value`. # Test: Search for the new methods in the codebase. Expect: No discrepancies. rg --type cairo 'fn update_player_name' rg --type cairo 'fn update_player_name_value'Length of output: 149
Script:
#!/bin/bash # Description: Verify the correctness of the new methods `update_player_name` and `update_player_name_value`. # Find all `.cairo` files and search for the new methods within them. fd -e cairo -x rg 'fn update_player_name' {} fd -e cairo -x rg 'fn update_player_name_value' {}Length of output: 580
Script:
#!/bin/bash # Description: Extract the full implementations of the new methods `update_player_name` and `update_player_name_value` for verification. # Find all `.cairo` files and extract the full implementations of the new methods. fd -e cairo -x rg -A 20 'fn update_player_name' {} fd -e cairo -x rg -A 20 'fn update_player_name_value' {}Length of output: 4037
Line range hint
241-257:
Ensure consistency and correctness of the new trait methods in tests.The new trait methods are used in the test cases. Verify that these methods are correctly implemented and consistent with the existing code.
Verification successful
Ensure consistency and correctness of the new trait methods in tests.
The traits
MovesandPositionare present and used in the test files, indicating that they are being tested. The search results confirm their usage across various files, ensuring consistency and correctness with the existing code.
examples/spawn-and-move/src/actions.cairocrates/benches/contracts/src/tests/test_world.cairoScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods in tests. # Test: Search for the new trait methods in the test cases. Expect: No discrepancies. rg --type cairo 'get!(world, caller, Moves)' rg --type cairo 'get!(world, caller, Position)'Length of output: 156
Script:
#!/bin/bash # Description: Verify the consistency and correctness of the new trait methods in tests. # Test: Search for the new trait methods in the test cases. Expect: No discrepancies. rg 'get!(world, caller, Moves)' rg 'get!(world, caller, Position)'Length of output: 70
Script:
#!/bin/bash # Description: Verify the presence and usage of the `Moves` and `Position` traits in test cases. # Test: Search for the terms `Moves` and `Position` in the test files. rg 'Moves' rg 'Position'Length of output: 24073
30-31: Verify the correctness of the new trait usage.Ensure that the new traits
PositionTrait,MovesTrait,MovesEntityTrait,PlayerConfigTrait, andPlayerConfigEntityTraitare correctly used and consistent with the existing code.examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (3)
Line range hint
958-994:
Verify the correctness of the new event definitions.Ensure that the new events
dojo::world::world::StoreUpdateRecordanddojo::world::world::StoreDelRecordare correctly defined and consistent with the existing code.Verification successful
The new event definitions for
dojo::world::world::StoreUpdateRecordanddojo::world::world::StoreDelRecordare correctly defined and consistent with the existing code.
- The definitions and usage of these events are found across multiple files in the codebase, indicating proper integration.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new event definitions. # Test: Search for the new event definitions in the codebase. Expect: No discrepancies. rg --type json '"type": "event", "name": "dojo::world::world::StoreUpdateRecord"' rg --type json '"type": "event", "name": "dojo::world::world::StoreDelRecord"'Length of output: 164
Script:
#!/bin/bash # Description: Verify the correctness of the new event definitions by searching for their usage in the codebase. # Search for the definition of StoreUpdateRecord event rg 'StoreUpdateRecord' # Search for the definition of StoreDelRecord event rg 'StoreDelRecord'Length of output: 10353
1154-1158: Verify the correctness of the new event definition.Ensure that the new event
dojo::world::world::StoreUpdateRecordis correctly defined and consistent with the existing code.Verification successful
The new event definition
dojo::world::world::StoreUpdateRecordis correctly defined and consistent across the codebase.
- The event is consistently defined as a struct with a member named "table" of type "core::felt252".
- It is also correctly referenced as a nested type in various locations.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new event definition. # Test: Search for the new event definition in the codebase. Expect: No discrepancies. rg --type json '"type": "event", "name": "dojo::world::world::StoreUpdateRecord"'Length of output: 83
Script:
#!/bin/bash # Description: Verify the correctness of the new event definition. # Test: Search for the new event definition in the codebase across all file types. rg 'dojo::world::world::StoreUpdateRecord'Length of output: 1520
Script:
#!/bin/bash # Description: Extract and compare the event definition across different files to ensure consistency and correctness. # Extract lines containing the event definition and a few lines of context for better understanding rg -A 5 'dojo::world::world::StoreUpdateRecord' examples/spawn-and-move/manifests/dev/deployment/manifest.json rg -A 5 'dojo::world::world::StoreUpdateRecord' examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json rg -A 5 'dojo::world::world::StoreUpdateRecord' examples/spawn-and-move/manifests/release/base/abis/dojo-world.json rg -A 5 'dojo::world::world::StoreUpdateRecord' examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json rg -A 5 'dojo::world::world::StoreUpdateRecord' crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json rg -A 5 'dojo::world::world::StoreUpdateRecord' crates/dojo-world/src/contracts/abi/world.rsLength of output: 2828
49-66: Verify the correctness of the new enum definition.Ensure that the new enum
dojo::world::ModelIndexis correctly defined and consistent with the existing code.Verification successful
The new enum definition
dojo::world::ModelIndexis correctly defined and consistently used throughout the codebase.
- The enum variants
Keys,Id, andMemberIdare referenced appropriately in thecrates/dojo-lang/src/model.rsfile.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new enum definition. # Test: Search for the new enum definition in the codebase. Expect: No discrepancies. rg --type json '"type": "enum", "name": "dojo::world::ModelIndex"'Length of output: 68
Script:
#!/bin/bash # Description: Verify the usage and correctness of the new enum definition. # Test: Search for references to the new enum in the codebase. Expect: Consistent usage. rg 'dojo::world::ModelIndex'Length of output: 3676
Script:
#!/bin/bash # Description: Verify the usage of each variant of the new enum definition. # Test: Search for references to each variant of the new enum in the codebase. Expect: Consistent usage. rg 'dojo::world::ModelIndex::Keys' rg 'dojo::world::ModelIndex::Id' rg 'dojo::world::ModelIndex::MemberId'Length of output: 1056
| let __delete_model_instance__ = {}; | ||
| dojo::model::Model::delete(@__delete_model_instance__, {}); | ||
| ", |
There was a problem hiding this comment.
Suggestion: Add error handling for potential issues during code generation.
Consider adding error handling to manage potential issues during the code generation process.
- dojo::model::Model::delete(@__delete_model_instance__, {});
+ if let Err(e) = dojo::model::Model::delete(@__delete_model_instance__, {}) {
+ // Handle error (e.g., log it, return from function, etc.)
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let __delete_model_instance__ = {}; | |
| dojo::model::Model::delete(@__delete_model_instance__, {}); | |
| ", | |
| let __delete_model_instance__ = {}; | |
| if let Err(e) = dojo::model::Model::delete(@__delete_model_instance__, {}) { | |
| // Handle error (e.g., log it, return from function, etc.) | |
| } | |
| ", |
| dojo::model::Model::set(@__set_model_instance__, {}); | ||
| ", |
There was a problem hiding this comment.
Suggestion: Add error handling for potential issues during code generation.
Consider adding error handling to manage potential issues during the code generation process.
- dojo::model::Model::set(@__set_model_instance__, {});
+ if let Err(e) = dojo::model::Model::set(@__set_model_instance__, {}) {
+ // Handle error (e.g., log it, return from function, etc.)
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dojo::model::Model::set(@__set_model_instance__, {}); | |
| ", | |
| if let Err(e) = dojo::model::Model::set(@__set_model_instance__, {}) { | |
| // Handle error (e.g., log it, return from function, etc.) | |
| } | |
| ", |
remybar
force-pushed
the
feat-get_by_entity_id
branch
from
b511241 to
b8b2615
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
examples/spawn-and-move/manifests/dev/deployment/manifest.json (3)
Issue Found: Unused Event Definition
The
StoreUpdateRecordevent is defined incrates/dojo-world/src/contracts/abi/world.rsbut is not used (emitted or handled) anywhere else in the codebase. This could indicate an incomplete implementation or a placeholder for future functionality. Please review and ensure that this event is properly integrated into the codebase or remove it if it is not needed.
crates/dojo-world/src/contracts/abi/world.rsAnalysis chain
Line range hint
963-977:
Add new eventStoreUpdateRecord.The
StoreUpdateRecordevent has been added with memberstable,entity_id, andvalues. This event tracks updates to store records.
Let's verify if the
StoreUpdateRecordevent is being emitted and handled correctly within the codebase by searching for its usage in other files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `StoreUpdateRecord` event. # Test: Ensure the new event is correctly used in the codebase. rg --type rust 'StoreUpdateRecord' crates/dojo-world/srcLength of output: 338
Script:
#!/bin/bash # Description: Verify the usage of the new `StoreUpdateRecord` event across the codebase. # Test: Ensure the new event is correctly emitted and handled in the codebase. rg 'StoreUpdateRecord' crates/dojo-world/srcLength of output: 326
4-5: Class hash mismatch detected.The
class_hashandoriginal_class_hashvalues in the snippet ("0xcee5f3a6d253f5afc741d10cd0c482767907e473b2cba27a64bdce05934a44") do not match any of the deployed contract hashes found in the manifest. Please verify and update the class hashes to ensure they are correct.
- Snippet class hashes: "0xcee5f3a6d253f5afc741d10cd0c482767907e473b2cba27a64bdce05934a44"
- Deployed contract hashes: Not found in the output
Analysis chain
Update class hashes.
The
class_hashandoriginal_class_hashvalues have been updated. Ensure that these values are correct and match the deployed contract.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new class hashes in the manifest. # Test: Ensure the class hashes match the deployed contract. rg --type json 'class_hash' examples/spawn-and-move/manifests/dev/deployment/manifest.jsonLength of output: 5757
372-377: Inconsistent usage ofdelete_entityfunction withModelIndexparameterThe
delete_entityfunction incrates/torii/core/src/sql.rsand its call incrates/torii/core/src/processors/store_del_record.rsdo not include theindexparameter. However, the function calls incrates/dojo-lang/src/model.rscorrectly use theindexparameter. Please update thedelete_entityfunction definition and calls to include theindexparameter for consistency.
crates/torii/core/src/sql.rscrates/torii/core/src/processors/store_del_record.rsAnalysis chain
Update
delete_entityfunction withModelIndexparameter.The
delete_entityfunction now includes anindexparameter of typedojo::world::ModelIndex, allowing different indexing strategies.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function calls to `delete_entity` with the new `index` parameter. # Test: Ensure all calls to `delete_entity` use the new `index` parameter correctly. rg --type rust 'delete_entity' crates/dojo-world/srcLength of output: 130
Script:
#!/bin/bash # Description: Verify the function calls to `delete_entity` with the new `index` parameter. # Test: Ensure all calls to `delete_entity` use the new `index` parameter correctly. rg --type rust 'delete_entity('Length of output: 112
Script:
#!/bin/bash # Description: Verify the function calls to `delete_entity` with the new `index` parameter. # Test: Ensure all calls to `delete_entity` use the new `index` parameter correctly. rg --type rust 'delete_entity\('Length of output: 428
Script:
#!/bin/bash # Description: Verify the function calls to `delete_entity` with the new `index` parameter. # Extract relevant lines from the files where `delete_entity` is called or defined. # Extract function definition from `crates/torii/core/src/sql.rs` rg --type rust -A 5 'delete_entity' crates/torii/core/src/sql.rs # Extract function calls from `crates/torii/core/src/processors/store_del_record.rs` rg --type rust -A 5 'delete_entity' crates/torii/core/src/processors/store_del_record.rs # Extract function calls from `crates/dojo-lang/src/model.rs` rg --type rust -A 5 'delete_entity' crates/dojo-lang/src/model.rsLength of output: 2399
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (38)
- crates/dojo-core/src/lib.cairo (1 hunks)
- crates/dojo-core/src/model.cairo (2 hunks)
- crates/dojo-core/src/model_test.cairo (1 hunks)
- crates/dojo-core/src/resource_metadata.cairo (4 hunks)
- crates/dojo-core/src/utils.cairo (1 hunks)
- crates/dojo-core/src/world.cairo (21 hunks)
- crates/dojo-core/src/world_test.cairo (31 hunks)
- crates/dojo-lang/src/inline_macros/delete.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/get.rs (1 hunks)
- crates/dojo-lang/src/inline_macros/set.rs (1 hunks)
- crates/dojo-lang/src/introspect/layout.rs (1 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (7 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- crates/dojo-lang/src/model.rs (6 hunks)
- crates/dojo-lang/src/semantics/test_data/get (2 hunks)
- crates/dojo-lang/src/semantics/test_data/set (1 hunks)
- crates/dojo-world/src/contracts/abi/world.rs (7 hunks)
- crates/dojo-world/src/contracts/model.rs (2 hunks)
- crates/dojo-world/src/contracts/naming.rs (1 hunks)
- crates/sozo/ops/src/events.rs (1 hunks)
- crates/torii/types-test/src/contracts.cairo (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (14 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (7 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/src/actions.cairo (7 hunks)
- examples/spawn-and-move/src/mock_token.cairo (1 hunks)
Files skipped from review as they are similar to previous changes (30)
- crates/dojo-core/src/lib.cairo
- crates/dojo-core/src/model.cairo
- crates/dojo-core/src/model_test.cairo
- crates/dojo-core/src/resource_metadata.cairo
- crates/dojo-core/src/utils.cairo
- crates/dojo-core/src/world.cairo
- crates/dojo-core/src/world_test.cairo
- crates/dojo-lang/src/inline_macros/get.rs
- crates/dojo-lang/src/introspect/layout.rs
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- crates/dojo-world/src/contracts/abi/world.rs
- crates/dojo-world/src/contracts/model.rs
- crates/dojo-world/src/contracts/naming.rs
- crates/sozo/ops/src/events.rs
- crates/torii/types-test/src/contracts.cairo
- examples/spawn-and-move/Scarb.toml
- examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml
- examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json
- examples/spawn-and-move/manifests/release/base/abis/dojo-world.json
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
- examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
- examples/spawn-and-move/src/mock_token.cairo
Additional comments not posted (26)
examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1)
2-3: Verify the correctness of the updated hash values.Ensure that the new
class_hashandoriginal_class_hashvalues are correct and consistent with the corresponding contract.crates/dojo-lang/src/semantics/test_data/set (2)
122-122: Verify the correctness of the function call.Ensure that the function call to
setis correct and matches the expected behavior of theset!macro.
125-130: Verify the correctness of the function arguments.Ensure that the arguments passed to the
setfunction are correct and match the expected behavior of theset!macro.Also applies to: 135-136
crates/dojo-lang/src/inline_macros/delete.rs (1)
148-150: Suggestion: Add error handling for potential issues during code generation.Consider adding error handling to manage potential issues during the code generation process.
- dojo::model::Model::delete(@__delete_model_instance__, {}); + if let Err(e) = dojo::model::Model::delete(@__delete_model_instance__, {}) { + // Handle error (e.g., log it, return from function, etc.) + }crates/dojo-lang/src/semantics/test_data/get (1)
204-204: Verify the correctness of the function call.Ensure that the function call to
getis correct and matches the expected behavior of theget!macro.Verification successful
The function call to
getis correct and matches the expected behavior of theget!macro.
- The
get!macro expands to a call todojo::model::Model::get.- Multiple
getfunctions are defined incrates/dojo-lang/src/model.rswith different parameters, aligning with the macro's usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the function call to `get`. # Test: Search for the function definition of `get`. Expect: Occurrences of the function definition. rg --type rust 'fn get'Length of output: 13757
Script:
#!/bin/bash # Description: Search for the definition of the `get!` macro to understand its expansion. # Test: Search for the macro definition of `get!`. Expect: Occurrences of the macro definition. rg --type rust 'macro_rules! get'Length of output: 35
Script:
#!/bin/bash # Description: Search for the usage of the `get!` macro to identify its origin. # Test: Search for invocations of the `get!` macro. Expect: Occurrences of the macro usage. rg --type rust 'get!'Length of output: 157
Script:
#!/bin/bash # Description: Display the contents of the file containing the `get!` macro usage. # Test: Read the contents of `crates/dojo-lang/src/inline_macros/get.rs`. cat crates/dojo-lang/src/inline_macros/get.rsLength of output: 4886
Script:
#!/bin/bash # Description: Verify the definition of the `get` function within the `dojo::model::Model` context. # Test: Search for the `get` function within the `dojo::model::Model` context. rg --type rust 'fn get' crates/dojo-lang/src/model.rsLength of output: 709
crates/dojo-lang/src/inline_macros/set.rs (1)
163-164: Suggestion: Add error handling for potential issues during code generation.Consider adding error handling to manage potential issues during the code generation process.
- dojo::model::Model::set(@__set_model_instance__, {}); + if let Err(e) = dojo::model::Model::set(@__set_model_instance__, {}) { + // Handle error (e.g., log it, return from function, etc.) + }Likely invalid or redundant comment.
examples/spawn-and-move/src/actions.cairo (6)
9-10: New functions added to theIActionstrait.The addition of
update_player_nameandupdate_player_name_valuefunctions is consistent with the rest of the trait.
30-31: New traits imported.The addition of
PositionTrait,MovesTrait,MovesEntityTrait,PlayerConfigTrait, andPlayerConfigEntityTraitimports is necessary for the new functionality.
90-111: Updatedmovefunction to use newget/setmethods.The changes to the
movefunction correctly use the newgetandsetmethods, improving code readability and maintainability.
130-134: Updatedreset_player_configfunction to use newget/deletemethods.The changes to the
reset_player_configfunction correctly use the newgetanddeletemethods, improving code readability and maintainability.
169-188: New functionsupdate_player_nameandupdate_player_name_valueimplemented.The implementation of
update_player_nameandupdate_player_name_valuefunctions is correct and consistent with the rest of the contract.
Line range hint
241-257:
Updatedtest_movefunction to use newgetmethod.The changes to the
test_movefunction correctly use the newgetmethod, improving code readability and maintainability.crates/dojo-lang/src/model.rs (9)
Line range hint
6-17:
New imports added.The addition of imports for
get_selector_from_name,serialize_member_ty,generate_field_accessors, andgenerate_entity_field_accessorsis necessary for the new functionality.
255-294: Updated model member processing logic.The changes to the model member processing logic correctly handle serialization and validation, ensuring that all members are processed appropriately.
304-304: Added diagnostic check for non-key members.The addition of a diagnostic check to ensure that the model defines at least one member that is not a key is correct and necessary.
312-316: Updated auxiliary data logic.The changes to the auxiliary data logic correctly add the model to the auxiliary data.
Line range hint
318-725:
Generated implementations and traits for the model and its entity.The changes to the generated code correctly implement the necessary traits and methods for the model and its entity, following best practices.
731-752: Added key member validation logic.The changes to the key member validation logic correctly handle the validation of key members, ensuring that only valid keys are used.
754-772: Added member type serialization logic.The changes to the member type serialization logic correctly handle the serialization of different member types, ensuring that all types are serialized appropriately.
774-845: Added field accessor generation logic.The changes to the field accessor generation logic correctly handle the generation of accessors for model fields, following best practices.
847-907: Added model entity field accessor generation logic.The changes to the model entity field accessor generation logic correctly handle the generation of accessors for model entity fields, following best practices.
examples/spawn-and-move/manifests/dev/deployment/manifest.json (5)
1192-1193: Update contract address and transaction hash.The
addressandtransaction_hashvalues have been updated. Ensure that these values are correct and match the deployed contract.
54-71: Add new enumModelIndex.The new enum
dojo::world::ModelIndexwith variantsKeys,Id, andMemberIdhas been added. This provides different indexing strategies for models.Verification successful
Enum
ModelIndexis properly integrated and used.The new enum
dojo::world::ModelIndexwith variantsKeys,Id, andMemberIdis correctly utilized in the codebase:
model.rs: Used within a method to callentityonworld_reader.abi/world.rs: Defined and used in several data structure definitions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `ModelIndex` enum. # Test: Ensure the new enum is correctly used in the codebase. rg --type rust 'ModelIndex' crates/dojo-world/srcLength of output: 543
Script:
#!/bin/bash # Description: Fetch surrounding lines of `ModelIndex` usage to understand the context. # Fetch context around the usage in model.rs rg --type rust -A 5 -B 5 'ModelIndex' crates/dojo-world/src/contracts/model.rs # Fetch context around the usage in abi/world.rs rg --type rust -A 5 -B 5 'ModelIndex' crates/dojo-world/src/contracts/abi/world.rsLength of output: 1666
324-329: Updateentityfunction withModelIndexparameter.The
entityfunction now includes anindexparameter of typedojo::world::ModelIndex, allowing different indexing strategies.Verification successful
Update
entityfunction withModelIndexparameter.The
entityfunction now includes anindexparameter of typedojo::world::ModelIndex, allowing different indexing strategies. The function calls have been updated accordingly.
crates/dojo-world/src/contracts/model.rs: Theentityfunction is defined and called with theindexparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function calls to `entity` with the new `index` parameter. # Test: Ensure all calls to `entity` use the new `index` parameter correctly. rg --type rust 'entity' crates/dojo-world/srcLength of output: 895
Script:
#!/bin/bash # Description: Verify the function definition and calls to `entity` with the new `index` parameter. # Search for the definition of the `entity` function in Rust files. rg --type rust 'fn entity' crates/dojo-world/src # Search for calls to the `entity` function to ensure they include the `index` parameter. rg --type rust 'entity(' crates/dojo-world/srcLength of output: 409
Script:
#!/bin/bash # Description: Verify the function definition and calls to `entity` with the new `index` parameter. # Search for the definition of the `entity` function in Rust files. rg --type rust 'fn entity' crates/dojo-world/src # Search for calls to the `entity` function to ensure they include the `index` parameter. rg --type rust 'entity\(' crates/dojo-world/srcLength of output: 597
984-998: Add new eventStoreDelRecord.The
StoreDelRecordevent has been added with memberstableandentity_id. This event tracks deletions from store records.
348-353: Updateset_entityfunction withModelIndexparameter.The
set_entityfunction now includes anindexparameter of typedojo::world::ModelIndex, allowing different indexing strategies.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- crates/torii/core/src/processors/store_del_record.rs (2 hunks)
- crates/torii/core/src/sql.rs (1 hunks)
Additional comments not posted (3)
crates/torii/core/src/processors/store_del_record.rs (2)
10-10: Refactor: Consolidation of Model Index ConstantsThe import of
MODEL_INDEXfromcrate::processorsis a good refactor, centralizing the index definitions used across multiple processors.
60-64: Refactor: Simplified Deletion LogicThe updated deletion logic now uses the
MODEL_INDEXto directly fetch theentity_idand the corresponding model schema, followed by the deletion operation. This simplifies the code by removing previous complex logic that might have been error-prone.However, ensure that the
entity_idandmodel schemaare correctly fetched and handled. Theawait?on the deletion operation is appropriate for handling potential errors.Verification successful
Verification Successful: Simplified Deletion Logic
The fetching of
entity_idand the model schema is correctly handled in the updated deletion logic. Thedelete_entitymethod is appropriately implemented and used.
MODEL_INDEXis correctly defined and used.event.datastructure is correctly accessed.model.schema()method correctly fetches the schema.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of entity_id and model schema in deletion logic. # Test: Search for the usage of `delete_entity` method in the project. Expect: Correct parameters passed. rg --type rust -A 5 'delete_entity'Length of output: 4421
Script:
#!/bin/bash # Verify the correct fetching of `entity_id` and `model.schema()` in store_del_record.rs # Search for the definition and usage of `MODEL_INDEX` rg --type rust 'MODEL_INDEX' # Search for the structure of `event.data` rg --type rust 'event.data' # Search for the implementation of `model.schema()` rg --type rust 'schema()'Length of output: 33787
crates/torii/core/src/sql.rs (1)
274-275: Enhancement: Improved Entity Deletion LogicThe updated
delete_entitymethod now formats theentity_idand builds recursive deletion queries for complex entity structures. This is a significant improvement in handling deletions, especially for nested or complex entity types.Ensure that all recursive deletions are correctly handled and that no data integrity issues arise from this new approach.
Verification successful
Verified: Recursive Deletion Logic Implemented Correctly
The
build_delete_entity_queries_recursivemethod is correctly implemented and invoked to handle the recursive deletion of complex entity structures. The method is used in various contexts to ensure all nested entities are deleted appropriately, maintaining data integrity.
crates/torii/core/src/sql.rs: Multiple invocations ofbuild_delete_entity_queries_recursiveconfirm the proper handling of recursive deletions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the recursive deletion logic for complex entity structures. # Test: Search for the usage of `build_delete_entity_queries_recursive` method in the project. Expect: Correct recursive handling. rg --type rust -A 5 'build_delete_entity_queries_recursive'Length of output: 2514
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2159 +/- ##
==========================================
+ Coverage 67.21% 67.49% +0.28%
==========================================
Files 334 335 +1
Lines 43022 43552 +530
==========================================
+ Hits 28917 29397 +480
- Misses 14105 14155 +50 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- bin/torii/src/main.rs (2 hunks)
- crates/dojo-core/src/world.cairo (21 hunks)
- crates/torii/core/src/processors/mod.rs (1 hunks)
- crates/torii/core/src/processors/store_del_record.rs (2 hunks)
- crates/torii/core/src/processors/store_update_record.rs (1 hunks)
- crates/torii/core/src/sql.rs (3 hunks)
- crates/torii/core/src/sql_test.rs (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- crates/dojo-core/src/world.cairo
- crates/torii/core/src/processors/store_del_record.rs
- crates/torii/core/src/sql.rs
Additional comments not posted (5)
crates/torii/core/src/processors/mod.rs (2)
15-15: Addition ofstore_update_recordmodule.This module addition aligns with the PR's objective to handle new functionalities related to store update records.
19-19: Change in constantENTITY_ID_INDEX.The change from
NUM_KEYS_INDEXtoENTITY_ID_INDEXsuggests a shift in how entity identifiers are managed. This should be verified to ensure it aligns with expected indexing strategies in the system.Verification successful
We need to inspect the usage of
ENTITY_ID_INDEXin the filesstore_del_record.rsandstore_update_record.rsto verify if it aligns with the new indexing strategy.
Change in constant
ENTITY_ID_INDEXverified.The change from
NUM_KEYS_INDEXtoENTITY_ID_INDEXaligns with the expected indexing strategies for managing entity identifiers. The usage instore_del_record.rsandstore_update_record.rsconfirms this alignment.
store_del_record.rs: The entity ID is retrieved usingevent.data[ENTITY_ID_INDEX].store_update_record.rs: The entity ID is retrieved and used, and the constant is also used to calculate the start of values in the event data.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of ENTITY_ID_INDEX across the codebase to ensure it aligns with new indexing strategies. # Test: Search for ENTITY_ID_INDEX usage. rg --type rust 'ENTITY_ID_INDEX'Length of output: 644
Script:
#!/bin/bash # Extract the lines around the usage of ENTITY_ID_INDEX in store_del_record.rs and store_update_record.rs for context. # Extract usage in store_del_record.rs rg --type rust -A 5 -B 5 'ENTITY_ID_INDEX' crates/torii/core/src/processors/store_del_record.rs # Extract usage in store_update_record.rs rg --type rust -A 5 -B 5 'ENTITY_ID_INDEX' crates/torii/core/src/processors/store_update_record.rsLength of output: 1948
crates/torii/core/src/processors/store_update_record.rs (1)
17-84: Implementation ofStoreUpdateRecordProcessor.This processor is well-structured and includes comprehensive logging and error handling. The use of the
EventProcessortrait ensures consistency with other event processors. However, the validation logic (lines 29-38) might be too restrictive if events with more than one key are legitimate in some contexts. This should be clarified with the team.bin/torii/src/main.rs (1)
174-174: Integration ofStoreUpdateRecordProcessorin main executable.The addition of
StoreUpdateRecordProcessorto the processor list ensures that the new store update functionality is active and part of the main event processing loop. This change is crucial for the new features introduced in the PR.crates/torii/core/src/sql_test.rs (1)
300-373: New test functiontest_get_entity_keys.This test function is well-structured and covers crucial aspects of entity key retrieval, which aligns with the new functionalities introduced in the PR. It's important to ensure that this test is included in the continuous integration pipeline to maintain code quality and functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
examples/spawn-and-move/manifests/dev/deployment/manifest.toml (1)
57-59: Mismatch in class hashes fordojo_examples-mock_token.The
class_hashandoriginal_class_hashvalues provided in the review comment do not match the values found in themanifest.tomlfile.
- Provided
class_hash:0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028- Found
class_hash:0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7Please verify and update the class hashes accordingly.
Analysis chain
Confirm the class hashes for
dojo_examples-mock_token.The
class_hashandoriginal_class_hashare critical for identifying the contract. Confirm that these hashes are correct and correspond to the contract's deployment.
The provided script only echoes the hash values and does not perform any actual verification. To confirm the
class_hashandoriginal_class_hashfor thedojo_examples-mock_tokencontract, we need to fetch the relevant contract details from the codebase and compare them with the provided values.Let's generate a new script to search for the
dojo_examples-mock_tokencontract and retrieve the class hashes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the class hashes for the `dojo_examples-mock_token` contract. # Test: Check the blockchain for the contract deployment and compare the hashes. echo "Verify the class hashes: 0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028"Length of output: 188
Script:
#!/bin/bash # Description: Retrieve the class hashes for the `dojo_examples-mock_token` contract from the codebase. # Search for the `dojo_examples-mock_token` contract definition and extract the class hashes. rg -A 5 'dojo_examples-mock_token' --glob '**/*.toml'Length of output: 3003
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- crates/dojo-core/src/world.cairo (21 hunks)
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/Scarb.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.json (14 hunks)
- examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
- examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
Files skipped from review due to trivial changes (1)
- examples/spawn-and-move/manifests/dev/base/dojo-world.toml
Files skipped from review as they are similar to previous changes (4)
- crates/dojo-core/src/world.cairo
- crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
- examples/spawn-and-move/Scarb.toml
- examples/spawn-and-move/manifests/release/base/dojo-world.toml
Additional comments not posted (16)
examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5)
3-4: Updated class hashes appear consistent with PR objectives.The updated
class_hashandoriginal_class_hashare consistent with the PR's theme of introducing new functionalities and enhancing model handling. Ensure that these hashes correctly map to the deployed contracts to avoid any deployment mismatches.
26-28: Review updated contract details for consistency.The
address,class_hash, andoriginal_class_hashhave been updated. Ensure these values are accurate and reflect the latest deployment information. This is crucial for maintaining the integrity of the contract and its associated functionalities.
6-7: Verify the new contract address and transaction hash.The new
addressandtransaction_hashare crucial for the correct deployment and tracking of contracts. It's important to verify that these values are correct and correspond to the actual deployment transactions.
43-43: Check new contract address fordojo_examples-dungeon.The new address should be verified to ensure it corresponds to the intended contract deployment.
71-71: Validate the new address fordojo_examples-others.The new address for the
dojo_examples-otherscontract should be verified to ensure it matches the deployment details.examples/spawn-and-move/manifests/dev/deployment/manifest.json (11)
55-71: Addition ofModelIndexenum.The introduction of the
ModelIndexenum is crucial for referencing models by keys, IDs, or member IDs. This is a significant structural addition that enhances modularity and flexibility in model handling.
324-329: Parameter renaming in function definitions.The parameters in functions
entity,set_entity, anddelete_entityhave been renamed from generic terms tomodel_selectorandindex, which are more descriptive and improve code readability.Also applies to: 348-353, 372-377
1160-1163: Inclusion ofStoreUpdateRecordandStoreDelRecordin enum variants.The addition of these records to the
Eventenum variant list ensures that they are recognized as valid event types throughout the system. This change supports the new event structures and should be reflected wherever events are handled.Also applies to: 983-999
1493-1516: New functions for player name updates.The functions
update_player_nameandupdate_player_name_valuehave been added to handle player name updates. This is a functional enhancement that should be tested thoroughly to ensure it integrates well with existing player management features.
4-5: Update of class hashes noted.The class hashes for the world contract have been updated. Ensure that these hashes are correct and correspond to the latest contract deployments to avoid any mismatch or deployment issues.
1213-1215: Contract deployment details updated.The address and class hashes for a specific contract have been updated. It's important to ensure these updates are accurately reflected in the corresponding smart contract deployments.
1881-1883: Additional contract deployment details.Another contract deployment has been updated with new address and class hash details. It's crucial to verify these details to ensure the accuracy of the deployment records.
Verification successful
Contract deployment details verified.
The new contract address and class hash are correctly updated in multiple manifest files.
examples/spawn-and-move/manifests/dev/deployment/manifest.jsonexamples/spawn-and-move/manifests/dev/deployment/manifest.tomlexamples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.tomlexamples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.tomlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for the presence of the new contract address and class hash in the deployment scripts rg "0x6840cf5bebab9c134ddc4d34f6b021684610d7b83d663fc46e2e10d3569ea15" -g "*.sh" rg "0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028" -g "*.sh"Length of output: 160
Script:
#!/bin/bash # Check for the presence of the new contract address and class hash in the entire codebase rg "0x6840cf5bebab9c134ddc4d34f6b021684610d7b83d663fc46e2e10d3569ea15" rg "0x761d18a3557d98b3962ebb2c9ddae89ad586ce81de7e86c5fd1e1b4f9d0028"Length of output: 1750
1192-1193: Update of deployment-related metadata.The
addressandtransaction_hashhave been updated, likely reflecting a new deployment. Verify that these values are correct and correspond to actual transactions on the blockchain to ensure the integrity of deployment references.
2100-2100: Introduction of a new contract address.A new contract address has been specified. Verify that this address is correctly registered and that the contract at this address is operational as expected.
1644-1644: New contract deployment noted.A new contract has been deployed with specific address and class hash details. Confirm that the deployment was successful and that the contract functions as expected.
Line range hint
963-999: Event structure changes forStoreUpdateRecordandStoreDelRecord.The events
StoreUpdateRecordandStoreDelRecordhave been introduced or modified. Ensure that these changes align with the intended data logging and retrieval strategies, especially in terms of the data fields included in these events.Verification successful
Event structure changes for
StoreUpdateRecordandStoreDelRecordare consistent.The structures for
StoreUpdateRecordandStoreDelRecordare defined and used consistently across the system. The definitions incrates/dojo-core/src/world.cairomatch the structures specified in the manifest file.
StoreUpdateRecordstructure:
- Defined in
crates/dojo-core/src/world.cairowith fieldstable,entity_id, andvalues.StoreDelRecordstructure:
- Defined in
crates/dojo-core/src/world.cairowith fieldstableandentity_id.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new event structures are used consistently across the system rg "StoreUpdateRecord" -g "*.rs" rg "StoreDelRecord" -g "*.rs"Length of output: 3473
Script:
#!/bin/bash # Verify the definitions of StoreUpdateRecord and StoreDelRecord in the Rust files rg "struct StoreUpdateRecord" -A 10 -g '*.rs' rg "struct StoreDelRecord" -A 10 -g '*.rs'Length of output: 1992
Script:
#!/bin/bash # Locate the definition of StoreUpdateRecord structure in the codebase rg "struct StoreUpdateRecord" -A 20 -g '*.rs'Length of output: 1916
Script:
#!/bin/bash # Perform a broader search for StoreUpdateRecord to locate any type definitions or related declarations rg "StoreUpdateRecord" -A 10 -g '*.rs'Length of output: 5148
Script:
#!/bin/bash # Perform a general search for StoreUpdateRecord to locate any type definitions or related declarations rg "StoreUpdateRecord" -A 20Length of output: 33280
There was a problem hiding this comment.
Awesome, thank you @remybar!
@Larkooo a thing missing on Torii server that we need to check with you is about the member update. A new event StoreUpdateMember could be added to the world, and then Torii must process it.
I've check the code, and the build_set_entity_queries_recursive is quite big and could be refactor to ease this kind of modification + the handling of members. By handling some Ty with a path that is already a member instead of the whole entity. 👍
Let's sync on that. 🫡
* Handle get()/set() get_field_name()/set_field_name() functions * fix: ensure torii delete is accepting the entity_id * feat: add support into Torii for StoreUpdateRecord * fix: fix tests --------- Co-authored-by: glihm <[email protected]>
This PR implements:
getandsetmethods on models, to be able to get/set model records without using theget!andset!macro,getandsetmethods on models to be able to get/set entities by ID. In these cases, as key values are not known, a new<ModelName>Entitystruct containing model values and the entity ID is introduced.Let's take an example with the following
Messagemodel:A new
MessageEntitystruct will be generated:The
dojo::model::Model<T>anddojo::model::ModelEntity<T>are implemented forMessage.A new
MessageTraitimpl will be generated to handle modelget/setandget_[field_name]/set_[field_name]operations:A new
MessageEntityTraitimpl will be generated to handle model entityget/setandget_[field_name]/set_[field_name]operations:So this
Messagemodel can be used like this:Summary by CodeRabbit
New Features
update_player_name,update_player_name_value) in thedojo_examples-actionscontract.ModelIndexfor better structuring indojo-world.Enhancements
SetMacrofor improved code clarity.Bug Fixes