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.

Sign up for GitHub

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

Jump to bottom

Implement enabled flag for fps overlay #15246

Merged
merged 3 commits into from
Sep 17, 2024
Merged

Implement enabled flag for fps overlay #15246

merged 3 commits into from
Sep 17, 2024

Conversation

no-materials
Copy link
Contributor

@no-materials no-materials commented Sep 16, 2024 •
edited
Loading

Objective

Fixes #15223

Solution

Adds an enabled flag to the FpsOverlayConfig resource with a system that detects it's change, and adjusts the visibility of the overlay text entity.

Testing

I extended the fps_overlay example with the option to toggle the overlay. Run with:

cargo run --features="bevy_dev_tools" --example fps_overlay

@janhohenheim janhohenheim added D-Trivial Nice and easy! A great choice to get started with Bevy C-Examples An addition or correction to our examples A-UI Graphical user interfaces, styles, layouts, and widgets S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 16, 2024
alice-i-cecile
alice-i-cecile requested changes Sep 16, 2024
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small perf optimization then this LGTM.

@alice-i-cecile alice-i-cecile added D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 16, 2024
alice-i-cecile
alice-i-cecile requested changes Sep 17, 2024
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, you're totally right. That last change was unnecessary and should be reversed. I missed the run condition.

@no-materials
Copy link
Contributor Author

Whoops, you're totally right. That last change was unnecessary and should be reversed. I missed the run condition.

No problem! Reverted in 26ecd30

@mnmaita mnmaita added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Sep 17, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 17, 2024
Merged via the queue into bevyengine:main with commit b884f96 Sep 17, 2024
27 checks passed
@no-materials no-materials deleted the toggle-fps-overlay-display branch September 17, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnmaita mnmaita mnmaita approved these changes

@atornity atornity atornity approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add an option to disable FpsOverlayPlugin
5 participants
@no-materials @alice-i-cecile @atornity @mnmaita @janhohenheim