Mark ghost nodes as experimental and partially feature flag them#15961
Mark ghost nodes as experimental and partially feature flag them#15961alice-i-cecile merged 11 commits intobevyengine:mainfrom
Conversation
|
@villor can I get your review here? |
|
Small comment: You don't actually have to add an experimental folder. You can make an experimental module in the parent module, and then re-export the ghost node module's contents under it. This is what we did with other stuff. |
|
You added a new feature but didn't update the readme. Please run |
|
I think that an experimental folder is a bit cleaner :) |
villor
left a comment
There was a problem hiding this comment.
I’m glad we are keeping the ghost nodes! What’s an experimental game engine without experimentation 🤓
That’s a neat trick make it non-constructable while not having to change any of the traversal code.
I have no strong opinions on the file structure. Especially when there is only one experimental feature.
Solution looks good to me, spookiness included 🎃
|
You added a new feature but didn't update the readme. Please run |
|
Yes, it's thematically appropriate that ghost nodes rely on phantom data. I can't wait to see the halloween-flavored release notes! |
|
You added a new feature but didn't update the readme. Please run |
I was tempted to just use a boring |
bushrat011899
left a comment
There was a problem hiding this comment.
I think I have a cleaner solution that avoids the need for a constructor. Otherwise, I think this is a good idea.
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
This reverts commit 45f6aae.
|
@bushrat011899 doesn't work :) |
bushrat011899
left a comment
There was a problem hiding this comment.
Sad my suggestion didn't pan out, I forgot that a tuple-struct can be used like a function. Looks good to go!
Objective
As discussed in #15341, ghost nodes are a contentious and experimental feature. In the interest of enabling ecosystem experimentation, we've decided to keep them in Bevy 0.15.
That said, we don't use them internally, and don't expect third-party crates to support them. If the experimentation returns a negative result (they aren't very useful, an alternative design is preferred etc) they will be removed.
We should clearly communicate this status to users, and make sure that users don't use ghost nodes in their projects without a very clear understanding of what they're getting themselves into.
Solution
To make life easy for users (and Bevy),
GhostNodeand all associated helpers remain public and are always available.However, actually constructing these requires enabling a feature flag that's clearly marked as experimental. To do so, I've added a meaningless private field.
When the feature flag is enabled, our constructs (
newanddefault) can be used. I've added anewconstructor, which should be preferred overDefault::defaultas that can be readily deprecated, allowing us to prompt users to swap over to the much nicerGhostNodesyntax once this is a unit struct again.Full credit: this was mostly @cart's design: I'm just implementing it!
Testing
I've run the ghost_nodes example and it fails to compile without the feature flag. With the feature flag, it works fine :)