Rename WorldQueryData & WorldQueryFilter to QueryData & QueryFilter#10779
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
|
@ItsDoot @james-j-obrien review please :D |
|
@taizu-jin great PR description <3 Thanks for tackling this. |
|
So the "fix" 25b9d7d for missing_deref test was actually an opposite. Is there a reason why my test run had a different output there and I could fix that instead or I should've just ignored it because I didn't touch that part of the code? |
james-j-obrien
left a comment
There was a problem hiding this comment.
I'm a fan. The World prefix always looked odd to me (it's not used anywhere else) and seemed like a way to avoid conflicting with the Query type. Now that it's not a problem I see no reason not to do this.
| } | ||
|
|
||
| pub static WORLD_QUERY_DATA_ATTRIBUTE_NAME: &str = "world_query_data"; | ||
| pub static WORLD_QUERY_DATA_ATTRIBUTE_NAME: &str = "query_data"; |
There was a problem hiding this comment.
Perhaps remove WORLD here as well?
There was a problem hiding this comment.
I was contemplating this as well. How about derive_world_query_filter, derive_world_query_filter_impl fns and their helper structs? Does it make sense to also rename the source file as well?
There was a problem hiding this comment.
Imo it doesn't make sense to leave traces of the old naming that might mislead future contributors, but in the end these internal names/filenames aren't going to affect users so it's not a huge deal.
There was a problem hiding this comment.
Gotcha, I'm leaning on cleaning it up completely. Do you mind if I re-request for a review after the changes?
ItsDoot
left a comment
There was a problem hiding this comment.
Looks good as-is, as long as James' concerns are satisfied. All the suggestions I list here are just suggestions and not required for my approval.
Nilirad
left a comment
There was a problem hiding this comment.
I think WorldQueryDataFilter and WorldQueryDataAttributes should be changed. They are private, but still worth changing for consistency.
Also, module names and their respective files should be adapted too (e.g. world_query_data.rs → query_data.rs)
5dc8686 to
9e09aec
Compare
Nilirad
left a comment
There was a problem hiding this comment.
Review was tedious but straightforward. I think we can proceed.
…10782) # Objective Since #10776 split `WorldQuery` to `WorldQueryData` and `WorldQueryFilter`, it should be clear that the query is actually composed of two parts. It is not factually correct to call "query" only the data part. Therefore I suggest to rename the `Q` parameter to `D` in `Query` and related items. As far as I know, there shouldn't be breaking changes from renaming generic type parameters. ## Solution I used a combination of rust-analyzer go to reference and `Ctrl-F`ing various patterns to catch as many cases as possible. Hopefully I got them all. Feel free to check if you're concerned of me having missed some. ## Notes This and #10779 have many lines in common, so merging one will cause a lot of merge conflicts to the other. --------- Co-authored-by: Alice Cecile <[email protected]>
# Objective > Can anyone explain to me the reasoning of renaming all the types named Query to Data. I'm talking about this PR #10779 It doesn't make sense to me that a bunch of types that are used to run queries aren't named Query anymore. Like ViewQuery on the ViewNode is the type of the Query. I don't really understand the point of the rename, it just seems like it hides the fact that a query will run based on those types. [@IceSentry](https://discord.com/channels/691052431525675048/692572690833473578/1184946251431694387) ## Solution Revert several renames in #10779. ## Changelog - `ViewNode::ViewData` is now `ViewNode::ViewQuery` again. ## Migration Guide - This PR amends the migration guide in #10779 --------- Co-authored-by: atlas dostal <[email protected]>
Rename
WorldQueryData&WorldQueryFiltertoQueryData&QueryFilterFixes #10776
Solution
Traits
WorldQueryData&WorldQueryFilterwere renamed toQueryDataandQueryFilter, respectively. Related Trait types were also renamed.Changelog
WorldQueryDatahas been renamed toQueryData. Derive macro'sQueryDataattributeworld_query_datahas been renamed toquery_data.WorldQueryFilterhas been renamed toQueryFilter. Derive macro'sQueryFilterattributeworld_query_filterhas been renamed toquery_filter.ExtractComponenttypeQueryhas been renamed toData.GetBatchDatatypesQuery&QueryFilterhas been renamed toData&Filter, respectively.ExtractInstancetypeQueryhas been renamed toData.ViewNodetypeViewQueryhas been renamed toViewData.RenderCommandtypesViewWorldQuery&ItemWorldQueryhas been renamed toViewData&ItemData, respectively.Migration Guide
Note: if merged before 0.13 is released, this should instead modify the migration guide of #10776 with the updated names.
WorldQueryData&WorldQueryFiltertrait usages toQueryData&QueryFilterand their respective derive macro attributesworld_query_data&world_query_filtertoquery_data&query_filter.ExtractComponenttypeQuerytoData.GetBatchDatatypeQuerytoData.ExtractInstancetypeQuerytoData.ViewNodetypeViewQuerytoViewData'RenderCommandtypesViewWorldQuery&ItemWorldQuerytoViewData&ItemData, respectively.