Skip to content

Auto add default UI camera #3679

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

Closed
wants to merge 2 commits into from

Conversation

Wcubed
Copy link
Contributor

@Wcubed Wcubed commented Jan 15, 2022

Adds a system to PostStartup that spawns a default UI camera if it finds UI nodes but no camera to render them.

Partial solution to #1432.

Open question

When there are no UI elements present during startup, the system won't trigger. Elements added later on in the game won't therefore be visible.

One solution to this would be to move the new check from startup to some stage that runs every frame. Would this be overkill? And is there a possibility that a user might have UI nodes, but doesn't want them to be rendered at the moment?

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 15, 2022
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Jan 15, 2022
@alice-i-cecile
Copy link
Member

This is a neat idea, thanks for the contribution! Code quality and tests are excellent.

I'm of two minds about this idea. In favor:

  • this is a frustrating and mysterious error for new beginners
  • UI cameras are rarely customized
  • this significantly reduces boilerplate of quick examples, and makes adding debug text / elements to games much lower friction.
  • users can just despawn this camera if they really care

Against:

  • this is implicit, and somewhat surprising
  • it obscures the fact that UI cameras exists and can be configured

If we don't do this, I think we should print a warning instead, using the same schedule position as you have here. Overall, I think I've convinced myself that this is worth it.

@Wcubed could you add a resource that controls whether or not this is done, defaulting to true? That will make it simpler to opt-out of this if users really care.

@alice-i-cecile
Copy link
Member

One more note: I think this should be blocked on #1854 / #3635. Users almost always want to add a marker component to each of their cameras right now, and fetching just the UI camera without a marker is currently a nuisance.

@Wcubed
Copy link
Contributor Author

Wcubed commented Jan 16, 2022

Good points.

  • The option to print a warning was considered earlier, with cart mentioning that his ideal solution would be to auto-add the UI camera: Warn if there is no UI camera #1440 (comment).

  • When I add a resource, I could instead make it so it always adds the default UI camera, unless the user specifies it shouldn't. This way it will work regardless of whether there are UI elements at startup. Which removes that gotcha, and makes the situation: "the UI camera is just there, unless you don't want it".
    Or would that incur too much unnecessary overhead for those not using it, and not aware it spawns one by default?

  • I agree that this should be blocked on the camera marker components. I now see I don't actually distinguish between a 2d Ortographic camera, and the UI camera. So it should either check the camera name, or the new marker components.

@mockersf
Copy link
Member

I would rather have the UI renderer work without a camera by assuming one with the default value rather than spawn one that could conflict with the user

@Wcubed
Copy link
Contributor Author

Wcubed commented Jan 16, 2022

If I understand you correctly, you propose that if the ecs world does not contain a ui camera, the render system would assume in the background it has one with the default settings?

That would have less edge-cases or gotcha's, I think. Interestingly, it would make the UI camera a bit of a "config object". If it's not there, defaults are assumed, so it's existence would then serve to override those defaults. Not sure if that is the desired direction.

For the implementation I think this if let could be turned into a match, with the None case spawning a default camera into the comands. I'm not sure whether the extrarted entities are kept between frames, if so, that would only move the problem of creating a default camera to the extract phase, and the solution is probably somewhere else.

if let Some(camera_ui) = active_cameras.get(CAMERA_UI) {

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen X-Controversial There is active debate or serious implications around merging this PR labels May 16, 2022
@Weibye
Copy link
Contributor

Weibye commented Aug 1, 2022

I would rather have the UI renderer work without a camera by assuming one with the default value rather than spawn one that could conflict with the user

I agree with @mockersf here that this problem might be better solved by improving the design of other systems.

@mockersf
Copy link
Member

mockersf commented Aug 1, 2022

Since Bevy 0.8, there isn't a need for a UI camera anymore, and 2D/3D camera will display UI by default
https://bevyengine.org/news/bevy-0-8/#no-more-camerauibundle

@mockersf mockersf closed this Aug 1, 2022
@Wcubed Wcubed deleted the auto-add-default-ui-camera branch August 6, 2022 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants