dojoengine / dojo

Dojo is a toolchain for building provable games and autonomous worlds with Cairo
https://dojoengine.org
Apache License 2.0
407 stars 165 forks source link

[BUG] Derive `Introspect` doesn't work with enum with u64 . #1547

Closed zsluedem closed 6 months ago

zsluedem commented 7 months ago

Describe the bug A clear and concise description of what the bug is.

Derive Introspect doesn't work with enum with u64 .

To Reproduce Steps to reproduce the behavior:

[derive(Drop, Introspect, Copy, Serde)]
enum BuildingId{
    a: u64,
    b: u64
}
sozo build 

with error

error: Plugin diagnostic: Trait has no implementation in context: dojo::database::introspect::Introspect::<core::integer::u64>
 --> some.cairo[BuildingId]:25:73
                        @dojo::database::introspect::Introspect::<u64>::ty()
                                                                        ^^

error: Compilation failed.

Expected behavior A clear and concise description of what you expected to happen.

build should work

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

glihm commented 7 months ago

Hey @zsluedem, thank you very much for this good catch. Revising the code, it seems to be here: https://github.com/dojoengine/dojo/blob/ca3b018048ddbeaa3a78c8287143ba526e0f89a0/crates/dojo-lang/src/introspect.rs#L135-L144 Were we should be checking (as we do for struct members) if the type if a primitive or not.

Would you like to fix it? Feel free to ask any question.

glihm commented 7 months ago

hello @glihm, assign this task to me

Let's do it! Thank you for that.

Please consider adding also tests, it is done by adding a code to test in this section: https://github.com/dojoengine/dojo/blob/9ea17b47f1dba8f6085cac12191fea64d5777604/crates/dojo-lang/src/plugin_test_data/introspect#L6-L51

And then, you can run CAIRO_FIX_TESTS=1 cargo test --package dojo-lang to ensure cairo is regenerating the tests with expected output. :+1:

Don't hesitate if you have any question.

glihm commented 7 months ago

you mean introspect does not work with type u64

Yes it works but not in enum with a path (but it works with tuple). So you have to insert a code that ensures the path is also supported.

So in the code here: https://github.com/dojoengine/dojo/blob/afa4e93688d3c75de0cf2469d226b06ca85dbdd4/crates/dojo-lang/src/introspect.rs#L181-L192

You will have to add a condition to check if the type is a primitive. If yes, you can do the exact same thing as it's done above for tuple.

Does it make sense?

glihm commented 7 months ago

@g4titan how are you doing? Any update on this?

zsluedem commented 6 months ago

@remybar Solved this one?

remybar commented 6 months ago

@remybar Solved this one?

Yes ser, issue solved !