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

Add support for full component paths, to avoid collisions from short_names #166

Open
janhohenheim opened this issue Mar 6, 2024 · 13 comments · May be fixed by #170
Open

Add support for full component paths, to avoid collisions from short_names #166

janhohenheim opened this issue Mar 6, 2024 · 13 comments · May be fixed by #170

Comments

@janhohenheim
Copy link
Contributor

Say I have two quests Foo and Bar. They might both have a component Objective, but in order to disambiguate them in the Blender addon, I have to name them FooObjective and BarObjective. If we instead had some kind of collapsible groupings, I would not have to do this.

@kaosat-dev
Copy link
Owner

Sorry @janhohenheim , I reread your message a few times, and I still don't understand either the issue or what you mean with the "collapsible grouping" ?

@janhohenheim
Copy link
Contributor Author

This:

Foo - Objective
Bar - Objective

@kaosat-dev
Copy link
Owner

Ok, but what is the actual issue ?

  • Are Foo & Bar entities, or components ?
  • What should the grouping be based on ?

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Mar 7, 2024

It doesn't really matter, as long as there is a way to register and differentiate two components with the same name.
One guaranteed unique identifier would be the full namespace, which would end up like this in the Blender UI:

bevy_core_pipeline
core_2d
camera_2d Camera2d
other_bevy_crates stuff
my_game
foo Objective
bar Objective

the obvious issue here is that the nesting can get quite deep for Bevy types. Maybe we'd have to filter those and put them all into a single flat group called "Bevy".

Alternatively, I can also imagine passing a group along as metadata for the registry export, e.g. as extension methods on App:

app
  .group_type::<foo::Objective>("Foo")
  .group_type::<bar::Objective>("Bar")

The downside here is that the registry is no longer a simple dump of reflection data.

@janhohenheim
Copy link
Contributor Author

My core issue is that I want to organize my Rust code into different modules that all don't know about each other. However, the Blender addon effectively forces me to use only unique names for components that I wish to use in the Blender UI, so there is a weird coupling between modules now. My modules quests::kill_the_rich_baron and quests::steal_the_money_from_the_vault cannot each have a component named Objective, even though in the context of their respective scopes, this is exactly what I'd want. Instead I have to have quests::kill_the_rich_baron::KillTheRichBaronObjective and quests::steal_the_money_from_the_vault::StealTheMoneyFromTheVaultObjective. This is repetitive, reads worse and is harder to refactor.

@janhohenheim
Copy link
Contributor Author

Does this make sense now? Sorry if what I wrote before was confusing :)

@kaosat-dev
Copy link
Owner

Ah yes, It makes sense now ! :)
Sadly this is way more than just a UI issue, it essentially boils down to the use of Component "short names", and it would be insanely hard to get away from it: (because I hit the limitations below multiple times)

  • the short names are the component names stored in gltf_extras , which are stored in Blender inside custom_properties
  • blender has a 63 chars limit for custom properties ... which is in most cases not enough to store full component paths
  • even IF there was a way to store the data differently, literally the whole workflow is centered on the use of custom properties, so it would be a HUGE task to rewrite everything both on the Blender & Bevy side

@janhohenheim
Copy link
Contributor Author

Good point :/

@kaosat-dev
Copy link
Owner

Some more thoughts:

  • the current Bevy_components UI is able to display more than one Component with the same shortname if they have different full "paths" (if you hover over them in the select box you can see the fully qualified path)
  • I have experimented a fair bit with hashing the (sometimes very long) full paths, but they usually exceed the 63 chars (I find it insane that there is such a limitation in Blender, that can even be circumvented if you are willing to compile Blender from code, and break all your blend files after that etc)
  • There is a hack-ish way to support this of the top of my head , but it feels...yikes : instead of storing the short name in custom properties, store the index (or a bevy side generated numeric index that does not change across changes to the registry...) of the component, which would require a lot less breaking changes

@janhohenheim
Copy link
Contributor Author

Apparently the same limit exists for other names as well: https://blenderartists.org/t/material-name-character-limit-63/1146620/18
This is really really dumb on Blender's side

@janhohenheim
Copy link
Contributor Author

janhohenheim commented Mar 7, 2024

As far as I can tell, the 63 char limit only exists on the names of the properties. What if the addon used only a single property called "bevy_components" that contained a RON array of all components with their full paths?
At least on the Bevy side that shouldn't be too much work.

@kaosat-dev
Copy link
Owner

Apparently the same limit exists for other names as well: https://blenderartists.org/t/material-name-character-limit-63/1146620/18 This is really really dumb on Blender's side

Yep, ran into that one multiple times while developing bevy_components (and the reason for the first breaking change), sigh

As far as I can tell, the 63 char limit only exists on the names of the properties. What if the addon used only a single property called "bevy_components" that contained a RON array of all components with their full paths? At least on the Bevy side that shouldn't be too much work.

...hmm packing it all into one could work, but yeah, waay too much work for now for all the rest.
I am already close to burning out on this whole project :D

@janhohenheim
Copy link
Contributor Author

Fair enough, it's not that important. As I said on Discord, I'm very happy with the workflow as-is, even if it never supported any additional features from now on ❤️

@kaosat-dev kaosat-dev changed the title Allow grouping components for Blender addon Add support for full component paths, to avoid collisions from short_names Mar 14, 2024
@kaosat-dev kaosat-dev linked a pull request May 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants