feat(dojo-bindgen): add namespace to unity bindgen #2155
Conversation
Larkooo
changed the title
WalkthroughThe changes focus on enhancing namespace handling in the 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/dojo-bindgen/src/plugins/unity/mod.rs (3 hunks)
Additional comments not posted (4)
crates/dojo-bindgen/src/plugins/unity/mod.rs (4)
6-6: Import statement looks good.The import of
get_namespace_from_tagis necessary for the new functionality.
187-212: Namespace inclusion informat_modelfunction looks good.The changes correctly wrap the class definition within the provided namespace.
261-261: Usage ofget_namespace_from_taglooks good.The change correctly extracts the namespace from the model tag.
181-181: Function signature change looks good.The addition of the
namespaceparameter is necessary for the new functionality.However, ensure that all function calls to
format_modelmatch the new signature.Verification successful
All function calls to
UnityPlugin::format_modelmatch the new signature.The function signature change has been correctly applied and verified.
- crates/dojo-bindgen/src/plugins/unity/mod.rs: The call matches the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `format_model` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type rust $'format_model'Length of output: 587
Script:
#!/bin/bash # Description: Verify all function calls to UnityPlugin::format_model match the new signature. # Test: Search for all calls to UnityPlugin::format_model. rg --type rust 'UnityPlugin::format_model'Length of output: 222
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2155 +/- ##
=======================================
Coverage 67.98% 67.99%
=======================================
Files 331 331
Lines 42665 42672 +7
=======================================
+ Hits 29006 29013 +7
Misses 13659 13659 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/dojo-bindgen/src/plugins/unity/mod.rs (3 hunks)
Additional comments not posted (2)
crates/dojo-bindgen/src/plugins/unity/mod.rs (2)
6-6: Import statement change approved.The addition of
get_namespace_from_tagto the import statement aligns with its usage in the code.
261-265: Ensureget_namespace_from_tagreturns a valid namespace.The usage of the
format_modelfunction with the newnamespaceparameter looks correct. Verify thatget_namespace_from_tagreturns a valid namespace string.Verification successful
Ensure
get_namespace_from_tagreturns a valid namespace.The
get_namespace_from_tagfunction correctly extracts and returns the namespace part of the tag. The usage of theformat_modelfunction with the newnamespaceparameter looks correct.
crates/dojo-world/src/contracts/naming.rs, lines 1-4Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify `get_namespace_from_tag` returns a valid namespace string. # Test: Search for the definition and usage of `get_namespace_from_tag`. Expect: Valid namespace strings. rg --type rust $'fn get_namespace_from_tag'Length of output: 139
Script:
#!/bin/bash # Extract the implementation of `get_namespace_from_tag` function from the specified file. ast-grep --lang rust --pattern 'fn get_namespace_from_tag($_) { $$$ }' crates/dojo-world/src/contracts/naming.rsLength of output: 114
Script:
#!/bin/bash # Extract the implementation of `get_namespace_from_tag` function from the specified file using ripgrep. rg -A 20 'fn get_namespace_from_tag' crates/dojo-world/src/contracts/naming.rsLength of output: 830
| fn format_model(namespace: &str, model: &Composite) -> String { | ||
| let fields = model | ||
| .inners | ||
| .iter() | ||
| .map(|field| { | ||
| format!( | ||
| "[ModelField(\"{}\")]\n public {} {};", | ||
| "[ModelField(\"{}\")]\n public {} {};", | ||
| field.name, | ||
| UnityPlugin::map_type(&field.token), | ||
| field.name, | ||
| ) | ||
| }) | ||
| .collect::<Vec<String>>() | ||
| .join("\n\n "); | ||
| .join("\n\n "); | ||
|
|
||
| format!( | ||
| " | ||
| // Model definition for `{}` model | ||
| public class {} : ModelInstance {{ | ||
| {} | ||
|
|
||
| // Start is called before the first frame update | ||
| void Start() {{ | ||
| }} | ||
|
|
||
| // Update is called once per frame | ||
| void Update() {{ | ||
| namespace {namespace} {{ | ||
| // Model definition for `{}` model | ||
| public class {} : ModelInstance {{ | ||
| {} | ||
|
|
||
| // Start is called before the first frame update | ||
| void Start() {{ | ||
| }} | ||
|
|
||
| // Update is called once per frame | ||
| void Update() {{ | ||
| }} | ||
| }} | ||
| }} | ||
|
|
There was a problem hiding this comment.
Add validation for the namespace parameter.
The namespace parameter should be validated to ensure it is not empty or null.
fn format_model(namespace: &str, model: &Composite) -> String {
+ assert!(!namespace.is_empty(), "Namespace cannot be empty");
+ assert!(namespace.chars().all(|c| c.is_alphanumeric() || c == '_'), "Namespace can only contain alphanumeric characters and underscores");
let fields = model
.inners
.iter()
.map(|field| {
format!(
"[ModelField(\"{}\")]\n public {} {};",
field.name,
UnityPlugin::map_type(&field.token),
field.name,
)
})
.collect::<Vec<String>>()
.join("\n\n ");
format!(
"
namespace {namespace} {{
// Model definition for `{}` model
public class {} : ModelInstance {{
{}
// Start is called before the first frame update
void Start() {{
}}
// Update is called once per frame
void Update() {{
}}
}}
}}
",
model.type_path,
model.type_name(),
fields
)
}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.
| fn format_model(namespace: &str, model: &Composite) -> String { | |
| let fields = model | |
| .inners | |
| .iter() | |
| .map(|field| { | |
| format!( | |
| "[ModelField(\"{}\")]\n public {} {};", | |
| "[ModelField(\"{}\")]\n public {} {};", | |
| field.name, | |
| UnityPlugin::map_type(&field.token), | |
| field.name, | |
| ) | |
| }) | |
| .collect::<Vec<String>>() | |
| .join("\n\n "); | |
| .join("\n\n "); | |
| format!( | |
| " | |
| // Model definition for `{}` model | |
| public class {} : ModelInstance {{ | |
| {} | |
| // Start is called before the first frame update | |
| void Start() {{ | |
| }} | |
| // Update is called once per frame | |
| void Update() {{ | |
| namespace {namespace} {{ | |
| // Model definition for `{}` model | |
| public class {} : ModelInstance {{ | |
| {} | |
| // Start is called before the first frame update | |
| void Start() {{ | |
| }} | |
| // Update is called once per frame | |
| void Update() {{ | |
| }} | |
| }} | |
| }} | |
| fn format_model(namespace: &str, model: &Composite) -> String { | |
| assert!(!namespace.is_empty(), "Namespace cannot be empty"); | |
| assert!(namespace.chars().all(|c| c.is_alphanumeric() || c == '_'), "Namespace can only contain alphanumeric characters and underscores"); | |
| let fields = model | |
| .inners | |
| .iter() | |
| .map(|field| { | |
| format!( | |
| "[ModelField(\"{}\")]\n public {} {};", | |
| field.name, | |
| UnityPlugin::map_type(&field.token), | |
| field.name, | |
| ) | |
| }) | |
| .collect::<Vec<String>>() | |
| .join("\n\n "); | |
| format!( | |
| " | |
| namespace {namespace} {{ | |
| // Model definition for `{}` model | |
| public class {} : ModelInstance {{ | |
| {} | |
| // Start is called before the first frame update | |
| void Start() {{ | |
| }} | |
| // Update is called once per frame | |
| void Update() {{ | |
| }} | |
| }} | |
| }} | |
| ", | |
| model.type_path, | |
| model.type_name(), | |
| fields | |
| ) | |
| } |
we use c# namespaces as the top level wrapper for models and SDKs will use reflection to figure out the namespace of the model & reconstruct the tag from that
Summary by CodeRabbit