Repository navigation
Replies: 8 comments 11 replies
|
I'm very open to breaking changes here. Power users will benefit a lot from reduced code gen, and cleaning up the story around proxy types is a huge simplification. |
|
If we're open to making substantial changes, I wholeheartedly agree that we should make them. The current API is bloated, complex, and riddle with tech debt. We can slim this down a lot if we're okay with completely overhauling the crate. I think we could take some ideas from https://github.com/soqb/perfect-reflect (@soqb's proof of concept for simplifying the reflection API). One of the main things I liked was how all the sub-traits (e.g., pub enum Value {
Dyn(Box<dyn Reflect>),
Seq(Vec<Value>),
Struct(FieldValues),
Enum(VariantValue),
}Because at the end of the day, that really is the rough shape of all Rust types. And even that API could be further simplified (e.g., And of course, there's lots more (some that have already had some amount of work put into them): proper reflection-based serialization, proper reflection hashing/comparisons, smart pointer reflection, better feature gating, etc. Not to mention that we can take the opportunity to break things up so things aren't as tightly coupled. This not only makes it harder for new users/contributors to understand the crate, it's also made it difficult to split up the crate as part of Modular Reflection. All this to say I am definitely onboard with rearchitecting. I have other thoughts but I'll post them in separate comments for discussion 🙂 |
|
I think with the prospect of rearchitecting, we should also take a moment to consider what we're architecting. Is this a general-purpose reflection API built for any Rust user? Is this a gamedev-focused reflection API meant mostly for users of Bevy (or maybe even other game engines as well)? There will be a lot of overlap here, of course, but I think we should design this with an intended audience in mind. And personally while I'd love to make this general-purpose, I think it would serve Bevy better in the long-run to design the crate for Bevy. Do we want custom attributes? Do we want a way to attach ECS or UI data to fields?1 How can we make reflection-driven deserialization easier in asset readers?1 How can we make it easier to hook data into the editor? How can we make it easier to interop with scripting languages? The editor and inspector work should really be driving a lot of our design decisions. And by not being as cautious to introduce Bevy-specific code into the crate2, we can make it easier for both our users to use and our contributors to contribute to. That being said, I also do value the notion of the crate being more useful outside of just Bevy. So I am torn a bit. We could also consider splitting the crate into (at least) two crates: one for general Rust use and another built on top of it for Bevy use. Footnotes
|
|
Can we look into a nursery program or something for this new architecture? It would be nice if we give it a cycle or two for users to experiment with it as a separate crate before we force them to update all of their reflection logic to something that may have still needed time to cook. |
@cart would it be possible for you to share snippets of your ideas and/or general design? Perhaps you're already close to finishing up your PoC, but if not, it may be worth sharing your thoughts early in case there's any pushback that would save you from wasting your time on a certain aspect of it. |
I think cutting down on monomorphization and replacing it with pointer/offset-based calculations is a good decision that should reduce codegen massively without impacting performance much. It would also be nice if reflection api exposed those offsets to the user.
I've actually tried compressing the path before and replacing it with a single string + compile-time computed offsets into it. From what I remember it gave ~1MB savings on native build, but when I tested it on wasm + compression the result was very negligible (because compression handles it very well it seems), so I never went ahead with making it into a PR.
Just wanted to note that currently in inspector workgroup we're experimenting with relying heavily on Also, not sure if it's in scope of this work, but supporting |
Are we planning to have these "dynamic value types" implement the |
|
Might be worth it to take #24561 into account in the design |
Uh oh!
There was an error while loading. Please reload this page.
I've been in a bit of a yak shave for the past couple of days. As part of my BSN asset work I'm trying to make the reflection parts as efficient as possible (ex: avoid boxing for primitive types using a
DynamicValueenum, don't do string matching more than once). MyDynamicValue/DynamicFieldsabstraction worked well for struct patching, but enums requireFromReflect, which meansDynamicFieldsneeds to implement reflect. This is doable, but it would require storing string names on theDynamicFieldsinstance (not ideal) and it would require matching on string names, as this is what FromReflect relies on (not ideal, as at this point we already have the field indices, which are faster to look up).PartialReflect/Reflectsplit, introducing unnecessary complexity. We can simplify this a lot if we make these "dynamic value types" a standalone API that works on top of the reflection APII understand that changing the reflection APIs is disruptive, but I strongly suspect that we are stuck in a local optimum at the moment. I won't push for these changes until I've proven that to be true / the cost of porting friction is outweighed by the benefits of the switch. This will of course be a discussion as well (once we have actual details to discuss). This post is largely just a heads up that I'm investigating this.
All reactions