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

Warn if there is no UI camera #1440

Closed

Conversation

jakobhellermann
Copy link
Contributor

@jakobhellermann jakobhellermann commented Feb 13, 2021

fixes the UI part of #1432.

A system is added to POST_STARTUP which checks whether the world contains any Nodes but no ui cam and logs

WARN bevy_ui: The world contains ui nodes but no ui camera. Consider spawning a UiCameraBundle.

To opt out, a ValidationConfig is added to bevy_core. IMO adding a separate resource for every future validation system (e.g. #1439) would quickly become too complicated.

@DJMcNab
Copy link
Member

DJMcNab commented Feb 13, 2021

I think opting out of this should be a setting on the UIPlugin, not in bevy_core. bevy_core should not be required to know about its dependents.

@jakobhellermann
Copy link
Contributor Author

I kinda agree, but on the other hand I feel like one central place to configure these kinds of warnings would be nice.

Since the UiPlugin is usually initialized using DefaultPlugins, how would you set this on the UiPlugin?

@jakobhellermann
Copy link
Contributor Author

The only viable alternative I saw was adding UiValidationConfig, TransformValidationConfig, RenderValidationConfig etc.

@DJMcNab
Copy link
Member

DJMcNab commented Feb 13, 2021

Since the UiPlugin is usually initialized using DefaultPlugins, how would you set this on the UiPlugin?

You'd set it on the UIPlugin when you add the plugin group, using whichever incantation is correct to replace a plugin in a plugin group with itself but with additional data when you add it. I can't be bothered to look this up right now, and it's very unintuitive.

PluginGroup is a painful abstraction, but it's what we're stuck with at the moment, and we should work within it.

@jakobhellermann jakobhellermann marked this pull request as draft February 13, 2021 11:33
@cart
Copy link
Member

cart commented Feb 13, 2021

I think my ideal solution is to make the UiCamera completely optional (and fall back to a default value). Most people don't need control over the UI camera, so theres no real reason to dictate one (other than the fact that its easier to require one because that behavior is consistent with the other render passes). Given that theres some design tradeoffs here, maybe its better to just move directly to an "optional ui camera" model?

@cart
Copy link
Member

cart commented Feb 13, 2021

PluginGroup is a painful abstraction, but it's what we're stuck with at the moment, and we should work within it.

Ive been pushing us toward a "resource as configuration model" over "configuring plugins during construction model" for a number of reasons: #1382 (comment)

Imo users configuring Plugins in their constructor should be considered an anti-pattern.

@jakobhellermann
Copy link
Contributor Author

That's reasonable, I'll close this then.

@jakobhellermann jakobhellermann deleted the warn-on-no-ui-cam branch February 13, 2021 19:24
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 this pull request may close these issues.

3 participants