Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(lang): update convention for keys and values naming #2660

Merged
merged 9 commits into from
Nov 8, 2024
Merged

refactor(lang): update convention for keys and values naming #2660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3a701fb
Updated functions to clear up naming with serialized
bengineer42 Nov 8, 2024
5b155b1
Removed some redundecies in the code
bengineer42 Nov 8, 2024
477c24b
Merge branch 'main' into rename_serialized_keys_values/0
bengineer42 Nov 8, 2024
3037aed
Merge branch 'rename_serialized_keys_values/0' into remove-redundecies
bengineer42 Nov 8, 2024
4b7cb35
Removed some redundecies in the code
bengineer42 Nov 8, 2024
5877a35
updated benchmarks
bengineer42 Nov 8, 2024
e6bc73a
Merge pull request #2 from bengineer42/remove-redundecies
bengineer42 Nov 8, 2024
f9def8c
chore: bump scarb
glihm Nov 8, 2024
af95590
fix: rebuild test database
glihm Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ rpassword = "7.2.0"
rstest = "0.18.2"
rstest_reuse = "0.6.0"
salsa = "0.16.1"
scarb = { git = "https://github.com/dojoengine/scarb", rev = "b9965b7e2f0d97f2a97f18ca9a75bac541de7d84" }
scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "b9965b7e2f0d97f2a97f18ca9a75bac541de7d84" }
scarb = { git = "https://github.com/dojoengine/scarb", rev = "a6d3b5b17b288502fe9cc63c96c0ae22fd175857" }
scarb-ui = { git = "https://github.com/dojoengine/scarb", rev = "a6d3b5b17b288502fe9cc63c96c0ae22fd175857" }
semver = "1.0.5"
serde = { version = "1.0", features = [ "derive" ] }
serde_json = { version = "1.0", features = [ "arbitrary_precision" ] }
Expand Down
22 changes: 11 additions & 11 deletions crates/dojo/core-cairo-test/src/tests/benchmarks.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fn bench_simple_struct() {
gas.end("foo serialize");

let gas = GasCounterTrait::start();
let values: Span<felt252> = foo.values();
let values: Span<felt252> = foo.serialized_values();
gas.end("foo values");

assert(serialized.len() == 2, 'serialized wrong length');
Expand Down Expand Up @@ -249,7 +249,7 @@ fn test_struct_with_many_fields_fixed() {
gas.end("pos serialize");

let gas = GasCounterTrait::start();
let values: Span<felt252> = pos.values();
let values: Span<felt252> = pos.serialized_values();
gas.end("pos values");

assert(serialized.len() == values.len(), 'serialized not equal');
Expand All @@ -268,7 +268,7 @@ fn test_struct_with_many_fields_fixed() {
};

let gas = GasCounterTrait::start();
database::set('positions', '42', pos.values(), 0, layout);
database::set('positions', '42', pos.serialized_values(), 0, layout);
gas.end("pos db set");

let gas = GasCounterTrait::start();
Expand Down Expand Up @@ -297,7 +297,7 @@ fn bench_nested_struct_packed() {
gas.end("case serialize");

let gas = GasCounterTrait::start();
let values: Span<felt252> = case.values();
let values: Span<felt252> = case.serialized_values();
gas.end("case values");

assert(serialized.len() == values.len(), 'serialized not equal');
Expand Down Expand Up @@ -378,7 +378,7 @@ fn bench_complex_struct_packed() {
gas.end("chars serialize");

let gas = GasCounterTrait::start();
let values: Span<felt252> = char.values();
let values: Span<felt252> = char.serialized_values();
gas.end("chars values");

assert(serialized.len() == values.len(), 'serialized not equal');
Expand All @@ -398,7 +398,7 @@ fn bench_complex_struct_packed() {
};

let gas = GasCounterTrait::start();
database::set('chars', '42', char.values(), 0, layout);
database::set('chars', '42', char.serialized_values(), 0, layout);
gas.end("chars db set");

let gas = GasCounterTrait::start();
Expand Down Expand Up @@ -467,8 +467,8 @@ fn test_benchmark_set_entity() {
world
.set_entity(
model_selector: Model::<Case>::selector(DOJO_NSH),
index: ModelIndex::Keys(simple_entity_packed.keys()),
values: simple_entity_packed.values(),
index: ModelIndex::Keys(simple_entity_packed.serialized_keys()),
values: simple_entity_packed.serialized_values(),
Comment on lines +470 to +471
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ohayo sensei! Found an inconsistency in serialization!

While most changes correctly use the new serialization methods, there's an inconsistency at line 491 where keys() is used instead of serialized_keys(). This should be updated for consistency.

Apply this diff to fix the inconsistency:

-            index: ModelIndex::Keys(complex_entity.keys()),
+            index: ModelIndex::Keys(complex_entity.serialized_keys()),

Also applies to: 480-481, 491-491

layout: Model::<Case>::layout()
);
gas.end("World::SetEntity::SimplePacked");
Expand All @@ -477,8 +477,8 @@ fn test_benchmark_set_entity() {
world
.set_entity(
model_selector: Model::<CaseNotPacked>::selector(),
index: ModelIndex::Keys(simple_entity_not_packed.keys()),
values: simple_entity_not_packed.values(),
index: ModelIndex::Keys(simple_entity_not_packed.serialized_keys()),
values: simple_entity_not_packed.serialized_values(),
layout: Model::<CaseNotPacked>::layout()
);
gas.end("World::SetEntity::SimpleNotPacked");
Expand All @@ -488,7 +488,7 @@ fn test_benchmark_set_entity() {
.set_entity(
model_selector: Model::<ComplexModel>::selector(),
index: ModelIndex::Keys(complex_entity.keys()),
values: complex_entity.values(),
values: complex_entity.serialized_values(),
layout: Model::<ComplexModel>::layout()
);
gas.end("World::SetEntity::ComplexModel");
Expand Down
18 changes: 9 additions & 9 deletions crates/dojo/core-cairo-test/src/tests/model/model.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ fn test_values() {
let mvalues = FooValue { v1: 3, v2: 4 };
let expected_values = [3, 4].span();

let values = mvalues.values();
let values = mvalues.serialized_values();
assert!(expected_values == values);
}

#[test]
fn test_from_values() {
let mut values = [3, 4].span();

let model_values: Option<FooValue> = ModelValue::<FooValue>::from_values(1, ref values);
let model_values: Option<FooValue> = ModelValue::<FooValue>::from_serialized(values);
assert!(model_values.is_some());
let model_values = model_values.unwrap();
assert!(model_values.v1 == 3 && model_values.v2 == 4);
Expand All @@ -71,7 +71,7 @@ fn test_from_values() {
#[test]
fn test_from_values_bad_data() {
let mut values = [3].span();
let res: Option<FooValue> = ModelValue::<FooValue>::from_values(1, ref values);
let res: Option<FooValue> = ModelValue::<FooValue>::from_serialized(values);
assert!(res.is_none());
}

Expand All @@ -83,7 +83,7 @@ fn test_read_and_update_model_value() {
world.write_model(@foo);

let entity_id = foo.entity_id();
let mut model_value: FooValue = world.read_value(foo.key());
let mut model_value: FooValue = world.read_value(foo.keys());
assert_eq!(model_value.v1, foo.v1);
assert_eq!(model_value.v2, foo.v2);

Expand All @@ -92,7 +92,7 @@ fn test_read_and_update_model_value() {

world.write_value_from_id(entity_id, @model_value);

let read_values: FooValue = world.read_value(foo.key());
let read_values: FooValue = world.read_value(foo.keys());
assert!(read_values.v1 == model_value.v1 && read_values.v2 == model_value.v2);
}

Expand Down Expand Up @@ -153,20 +153,20 @@ fn test_delete_from_model() {
}

#[test]
fn test_model_ptr_from_key() {
fn test_model_ptr_from_keys() {
let mut world = spawn_foo_world();
let foo = Foo { k1: 1, k2: 2, v1: 3, v2: 4 };
let ptr = Model::<Foo>::ptr_from_key(foo.key());
let ptr = Model::<Foo>::ptr_from_keys(foo.keys());
world.write_model(@foo);
let v1 = world.read_member(ptr, selector!("v1"));
assert!(foo.v1 == v1);
}

#[test]
fn test_model_ptr_from_keys() {
fn test_model_ptr_from_serialized_keys() {
let mut world = spawn_foo_world();
let foo = Foo { k1: 1, k2: 2, v1: 3, v2: 4 };
let ptr = Model::<Foo>::ptr_from_keys(foo.keys());
let ptr = Model::<Foo>::ptr_from_serialized_keys(foo.serialized_keys());
world.write_model(@foo);
let v1 = world.read_member(ptr, selector!("v1"));
assert!(foo.v1 == v1);
Expand Down
7 changes: 5 additions & 2 deletions crates/dojo/core-cairo-test/src/tests/utils/key.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use dojo::utils::{entity_id_from_keys, combine_key};
use dojo::utils::{entity_id_from_serialized_keys, combine_key};

#[test]
fn test_entity_id_from_keys() {
let keys = [1, 2, 3].span();
assert(entity_id_from_keys(keys) == core::poseidon::poseidon_hash_span(keys), 'bad entity ID');
assert(
entity_id_from_serialized_keys(keys) == core::poseidon::poseidon_hash_span(keys),
'bad entity ID'
);
}

#[test]
Expand Down
Loading
Loading