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

Take example screenshots in CI #8488

Merged
merged 10 commits into from
May 1, 2023

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Apr 25, 2023

Objective

  • I want to take screenshots of examples in CI to help with validation of changes

Solution

  • Can override how much time is updated per frame
  • Can specify on which frame to take a screenshots
  • Save screenshots in CI

I reused the TimeUpdateStrategy::ManualDuration to be able to set the time update strategy to a fixed duration every frame. Its previous meaning didn't make much sense to me. This change makes it possible to have screenshots that are exactly the same across runs.

If this gets merged, I'll add visual comparison of screenshots between runs to ensure nothing gets broken

Migration Guide

  • TimeUpdateStrategy::ManualDuration meaning has changed. Instead of setting time to Instant::now() plus the given duration, it sets time to last update plus the given duration.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@github-actions
Copy link
Contributor

Example scene failed to run, please try running it locally and check the result.

@github-actions
Copy link
Contributor

Example load_gltf failed to run, please try running it locally and check the result.

@mockersf
Copy link
Member Author

Example screenshots can be found in the artefacts section of https://github.com/bevyengine/bevy/actions/runs/4800302635

@mockersf mockersf added the A-Build-System Related to build systems or continuous integration label Apr 25, 2023
@JMS55
Copy link
Contributor

JMS55 commented Apr 27, 2023

Would be awesome to use these CI-generated images in the examples page of the website :)

@mockersf
Copy link
Member Author

mockersf commented Apr 27, 2023

That will be the plan after the next release (if this gets merged)!

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels May 1, 2023
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.

I really like this idea in theory, and the API is good. I'm curious to see how this ends up being used and how new screenshots can be blessed, but I'm content with this as a first step.

Copy link
Member

@TheRawMeatball TheRawMeatball left a comment

Choose a reason for hiding this comment

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

There's definitely more work to be done, but this seems like a very good first step to me.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 1, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 1, 2023
Merged via the queue into bevyengine:main with commit 8070c29 May 1, 2023
@Selene-Amanita Selene-Amanita added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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
Development

Successfully merging this pull request may close these issues.

5 participants