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.

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

[BUG] [dojo-bindgen] [unity] Generated actions methods try to call FieldElements in calldata #2008

Closed
trevis-dev opened this issue May 27, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@trevis-dev
Copy link

trevis-dev commented May 27, 2024

Describe the bug
bindgen will generate code trying to call Inner on systems calldata elements.

To Reproduce
Steps to reproduce the behavior:

  1. clone dojo-starter
  2. run sozo build --unity
  3. Inspect Actions's Move in dojo_starter::systems::actions::actions.gen.cs which shows
calldata = new dojo.FieldElement[] {
    new FieldElement(direction).Inner()
}

Which doesn't compile.

Expected behavior
The resulting code should be

-    new FieldElement(direction).Inner()
+    new FieldElement(direction).Inner

Additional info
It seems that format_system always formats calldata with FieldElement({}).Inner()

match handled_tokens.iter().find(|t| t.type_name() == token.type_name()) {
Some(t) => {
// Need to flatten the struct members.
match t.r#type {
CompositeType::Struct => t
.inners
.iter()
.map(|field| {
format!(
"new FieldElement({}.{}).Inner()",
type_name, field.name
)
})
.collect::<Vec<String>>()
.join(",\n "),
_ => {
format!("new FieldElement({}).Inner()", type_name)
}
}
}
None => match UnityPlugin::map_type(type_name).as_str() {
"FieldElement" => format!("{}.Inner()", type_name),
_ => format!("new FieldElement({}).Inner()", type_name),
},
}
})

I'm guessing it should probably just be Inner everywhere. If confirmed, I'd be happy to make a PR.

@trevis-dev trevis-dev added the bug Something isn't working label May 27, 2024
@jancris100
Copy link

Hi I would like to work on this

@Larkooo Larkooo self-assigned this May 28, 2024
@Larkooo
Copy link
Collaborator

Larkooo commented May 28, 2024

Should be addressed by #1954

@Larkooo Larkooo closed this as completed May 28, 2024
glihm pushed a commit that referenced this issue May 29, 2024
* feat: new types in unity bindgen

* feat: add types support to typescript codegen too

* feat: handle token directly to get type name

* chore: correct type names

* chore: update to correct tuple  types

* fmt

* Update mod.rs

* Update mod.rs

* Update mod.rs

* feat: add support for complex enums to unity bindgen

* feat: support generic args in typescript

* fmt

* feat: newer tuple type for c#

* feat: value type name in record field

* feat: generic rust like enum for unity bindgen

* refacotr: optional generic args for record

* feat: add suppport for generic enums for typescript v2

* feat: update test & disable typescript

* fix: update unity systsem bindgen to use new felt getter #2008

* chore: clippy

* clean uop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants