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

CiTestingConfig is blocking bevy_dev_tools from using bevy_render #12356

Closed
pablo-lua opened this issue Mar 7, 2024 · 3 comments · Fixed by #12371
Closed

CiTestingConfig is blocking bevy_dev_tools from using bevy_render #12356

pablo-lua opened this issue Mar 7, 2024 · 3 comments · Fixed by #12371
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior

Comments

@pablo-lua
Copy link
Contributor

pablo-lua commented Mar 7, 2024

Bevy version

After #11341

What you did

Trying to import bevy_render or some crate that depends on bevy_render, like in #12354

What went wrong

This causes circular a dependence, because bevy_render depends internally on bevy_dev_tools

Additional information

Place where we use bevy_dev_tools in bevy_render

impl Plugin for ScreenshotPlugin {
   // . . .

    fn finish(&self, app: &mut bevy_app::App) {
        if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
            render_app.init_resource::<SpecializedRenderPipelines<ScreenshotToScreenPipeline>>();
        }

        #[cfg(feature = "bevy_ci_testing")]
        if app
            .world
            .contains_resource::<bevy_dev_tools::ci_testing::CiTestingConfig>()
        {
            app.add_systems(bevy_app::Update, ci_testing_screenshot_at);
        }
    }
}

#[cfg(feature = "bevy_ci_testing")]
fn ci_testing_screenshot_at(
    mut current_frame: Local<u32>,
    ci_testing_config: Res<bevy_dev_tools::ci_testing::CiTestingConfig>,
    mut screenshot_manager: ResMut<ScreenshotManager>,
    main_window: Query<Entity, With<bevy_window::PrimaryWindow>>,
) {
// . . .
}

Possible solutions

We can probably

  • Move related ci Test (probably used by screenshot example) into tests and adding this Ci system into there, so we can make sure that screenshots are still working. Then we can remove Ci from bevy_render
  • Move Ci back to where it was before, reverting the changes on this field, as we shouldn't really rely internally on nothing from inside bevy_dev_tools
  • Address this case in specific: If we are relying on screenshot to add this, maybe we can add the needed system somehow inside bevy_dev_tools?
@pablo-lua pablo-lua added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-Dev-Tools Tools used to debug Bevy applications. and removed S-Needs-Triage This issue needs to be labelled labels Mar 7, 2024
@alice-i-cecile
Copy link
Member

I think my preferred choice is actually to move the test out to our top-level tests folder.

Failing that I like just reverting that move.

@pablo-lua
Copy link
Contributor Author

I think we have it this way now just so we can ensure that Ci exits successfully on screenshot example, tbh. Not sure that it can be moved to tests at all, because its added to the ScreenshotPlugin when bevy_ci_testing feature is on.

@alice-i-cecile
Copy link
Member

Ugh right. Let's just revert then. FYI @mockersf.

github-merge-queue bot pushed a commit that referenced this issue Mar 7, 2024
# Objective

- Fix #12356
- better isolation of ci testing tools in dev tools instead of being in
various crates

## Solution

- Move the parts doing the work of ci testing to the dev tools
spectria-limina pushed a commit to spectria-limina/bevy that referenced this issue Mar 9, 2024
# Objective

- Fix bevyengine#12356
- better isolation of ci testing tools in dev tools instead of being in
various crates

## Solution

- Move the parts doing the work of ci testing to the dev tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants