Add loading_screen example #3618

Weasy666 · 2022-01-09T21:30:19Z

Objective

As stated here, bevyengine/bevy-website#236, the examples with big assets can be problematic, depending on the internet connection.

Solution

Add a loading_screen example and expose its system for other examples. This should (hopefully) be possible, with this.

This is a draft PR, because i first want to get feedback on the loading_screen example, if the way i do it, is the preferred one, or if there is a better one. Aaaaand...i didn't have more time today. Hope i can integrate it into load_gltf and update_gltf_scene example tomorrow. Are there more examples where a loading_screen makes sense?

alice-i-cecile · 2022-01-09T23:27:16Z

Interestingly, I wonder if this would actually make more sense as an optional Plugin, with doc comments saying "go look at this code!"

We want a way to add this logic to other examples quickly and without distraction, and I don't want to be importing things across examples.

The other option would be to do both, but that seems very prone to drifting divergence, which would really suck.

Weasy666 · 2022-01-10T10:57:26Z

I am not sure that this is a good idea for a plugin. I think there are a lot of different needs for a loading screen for different games, such as a progress bar, a background image (even changing ones), text for info or tips, some more animated stuff, etc. So a fully featured plugin is a different kind of beast....and i was under the impression, that the "official" stance is to not use any external plugins for examples in the bevy repo.

Whereas, with a utils/lib.rs module (or whatever name) in the examples-folder, you would be able to share code between examples. I think most people should be able to read and follow the import path to the actual code, if they need it.

#[path = "../common/lib.rs"]
mod common;

It's not ideal, but i don't know of any other way (except using a plugin) to do this.
If we don't want this (the code sharing), then i am ok with just adding it as a new example. I would just need to clean up the comments, i have seen that there are some "broken"/missing parts.

alice-i-cecile · 2022-01-10T17:12:15Z

and i was under the impression, that the "official" stance is to not use any external plugins for examples in the bevy repo.

That's correct. I was thinking making it an "official" optional plugin, in the sense of the Plugin trait.

I am not sure that this is a good idea for a plugin. I think there are a lot of different needs for a loading screen for different games, such as a progress bar, a background image (even changing ones), text for info or tips, some more animated stuff, etc. So a fully featured plugin is a different kind of beast

Hmm right: a plugin would be harder to extend than a code snippet that users can copy-paste.

If we don't want this (the code sharing), then i am ok with just adding it as a new example.

Yeah, I'm on the fence about this. @mockersf, do you have opinions?

@Weasy666 could you refactor this example to define a LoadingScreenPlugin and then use that within the example? That demonstrates the correct usage pattern cleanly, and should make it easier to copy-paste into user code / other examples.

Weasy666 · 2022-01-10T21:38:36Z

I can convert it into a plugin inside the loading_screen example, but the user would have to modify asset_listening_system and setup_loading_screen either way, at least when they want to change something on the loading screen...so i am not really sure if that will help in any way. Maybe i am misunderstanding what you mean. 😅

alice-i-cecile · 2022-01-11T14:20:26Z

I can convert it into a plugin inside the loading_screen example, but the user would have to modify asset_listening_system and setup_loading_screen either way, at least when they want to change something on the loading screen...so i am not really sure if that will help in any way

Yeah, this was my intent. The idea is that we can reduce the amount of modification needed: every user will want to make their own LoadingScreenPlugin wrapper. Rather than forcing them to write this boilerplate themselves each time, we can provide them the logic in a pre-wrapped form.

The intended workflow is "find example, copy-paste into their own app", so we should make that as easy as possible.

Weasy666 · 2022-01-11T14:40:27Z

Ok. Is it ok like this, or did you have something different in mind?

examples/asset/loading_screen.rs

IceSentry · 2022-01-11T22:01:39Z

examples/asset/loading_screen.rs

+        .add_plugins(DefaultPlugins)
+        // Loading screen plugin and its `asset_listening_system`
+        .add_plugin(plugin::SimpleLoadingScreenPlugin {
+            loading_state: GameState::Loading,


I'm not sure this needs to be internal state to the plugin. It's not really use anywhere inside of it. this could just be a state on the App.

Or, I think it's possible to move the asset_listening system inside the plugin. Let the plugin insert a LoadingState and have a system in the main example that simply listens to that State change

It's not really use anywhere inside of it.

It is used on line 117 and 120, so that the plugin "knows" at which state it has to work and when it needs to be closed. 🙂

bevy/examples/asset/loading_screen.rs

Lines 117 to 120 in d36ceef

SystemSet::on_update(self.loading_state.clone()).with_system(animate_spinner),

)

.add_system_set(

SystemSet::on_exit(self.loading_state.clone())

Or, I think it's possible to move the asset_listening system inside the plugin.

My initial intention for this example was, to later extract the plugin into a lib in the example folder, to share it with other examples. If something like shared example code is ok within Bevy. That would mean, that the plugin doesn't know to which AssetEvents it has to listen. That is the reason why i left the asset_listening_system in the main example. I wanted the plugin to take a Fn and store it internal, but the Plugin trait requires Send + Sync and the Fn would also need to be 'static which would make it really ugly. And using a generic with IntoSystemDescriptor<Params> bound would also not work, because it isn't Copy or Clone and as such can't be used as param for with_system, because we only get it as borrowed reference inside build. 😅

That said...if there is no chance, that this will happen, then there is no reason to not move asset_listening_system inside the plugin.
Regarding GameState...i thought, that we need a "global" state to make sure, that the systems run in the correct order and didn't want to impose a state to a game/app that uses the plugin.

For the state, I'd rather have a LoadingState that is controlled by the plugin entirely so that examples that need loading screens could simply do .add_system_set(SystemSet::on_enter(LoadingState::Done).with_system(game_system)). I believe states are already global by default if the struct holding the state is pub.

For the asset event issue, I didn't think about the fact that it was for specific asset types, but if it's only the gltf assets that are slow to load I wouldn't having it only for gltf. Or maybe a different plugin for each type.

Essentially, I would prefer if examples had as little code as possible related to loading assets and with the current approach it would be to verbose to use in other examples. It's still a decent example on it's own to showcase how a loading screen would work.

Hm...i'm not that deep into the stages and state stuff. What happens if an example, like alien_cake_addict, already has a state, can they be used together, is there a way to say, run LoadingState stuff first and afterwards run the GameState stuff?

Yeah...AssetEvents really are the problematic part in making this generic, without manually tracking (somehow registering) assets and their events in the plugin.

alice-i-cecile · 2022-01-11T23:55:36Z

Yep, this is exactly the direction I had in mind :) @IceSentry's comments are excellent, we can do a final polish pass once those are addressed.

examples/asset/loading_screen.rs

alice-i-cecile · 2022-04-18T18:32:01Z

bors try

alice-i-cecile · 2022-04-18T18:32:25Z

@Weasy666 can you pull this out of draft mode?

IceSentry · 2022-04-18T18:42:17Z

With the issue that extracts the camera controller to an example_utils crate, I think this one should follow a similar pattern

bors · 2022-04-18T18:54:17Z

try

Build succeeded:

Weasy666 · 2022-04-25T21:10:11Z

Sorry, for the late answer. This somehow slipped my mind. Oh...we now have an examples_utils crate? Cool! I'm planning to rebase and move this to the examples_utils crate in the course of this week. 🙂

IceSentry · 2022-04-25T21:32:03Z

Well, it's not merged right now, but #4458 introduces the example_utils crate

Nilirad · 2022-05-03T13:05:11Z

This PR adds a new example. Adding module and item level doc comments, as described in:

would be really useful to those who will browse examples.

Copypasta aside, this example is quite in line with the current standards. The only changes needed is to document the module instead of main and transform some regular comments here and there into doc comments.

…paration more clear.

…ipulation, because `loading_screen` query does not work because of system ordering.

alice-i-cecile · 2022-05-16T13:52:26Z

@bevyengine/docs-team can I get some eyes on this?

Nilirad

Not on docs team, but I'll do my best.

Suggested some changes to comply on #3951.

I also think there are too many comments. I would limit them to doc comments for all the items, and use normal comments just for things that are directly related to the scope of the example. Comments about layout, single and first, and such things are a little bit distracting IMO.

Nilirad · 2022-05-16T16:05:40Z

examples/asset/loading_screen.rs

+                .add_system_set(
+                    SystemSet::on_update(LoadingState::Loading).with_system(animate_spinner),
+                )
+                .add_system_set(
+                    SystemSet::on_update(LoadingState::Loading).with_system(asset_listening_system),
+                )


Not a system set expert, but wouldn't putting those two systems in the same set be the same?

Yes, would also work...i can only guess, that i had a good reason for doing this...naa...probably not 🙈

Nilirad · 2022-05-16T16:06:38Z

examples/asset/loading_screen.rs

+        .run();
+}
+
+// Here we prepare our gaming scene, meaning we setup our ingame camera and load our asset.


Suggested change

// Here we prepare our gaming scene, meaning we setup our ingame camera and load our asset.

/// Here we prepare our gaming scene, meaning we setup our ingame camera and load our asset.

Comments about items should be doc comments.

Nilirad · 2022-05-16T16:06:52Z

examples/asset/loading_screen.rs

+    });
+}
+
+// Your, or at least one of your, game systems.


Suggested change

// Your, or at least one of your, game systems.

/// Your, or at least one of your, game systems.

Nilirad · 2022-05-16T16:09:07Z

examples/asset/loading_screen.rs

+//########################################## Example of a loading screen plugin ##########################################//
+// You can use this as a template for your own project/game. Don't forget, that you also need a system,
+// like `asset_listening_system`, to track your loading progress and to transition into your playing state.


Suggested change

//########################################## Example of a loading screen plugin ##########################################//

// You can use this as a template for your own project/game. Don't forget, that you also need a system,

// like `asset_listening_system`, to track your loading progress and to transition into your playing state.

/// ########################################## Example of a loading screen plugin ##########################################//

/// You can use this as a template for your own project/game. Don't forget, that you also need a system,

/// like `asset_listening_system`, to track your loading progress and to transition into your playing state.

Honestly I'm not a fan of these decorations. I don't think we also have them in other parts of the engine.

Nilirad · 2022-05-16T16:11:50Z

examples/asset/loading_screen.rs

+    // Setup our simple loading screen, it uses an image as spinner and a loading text bellow it.
+    // INFO: you need to change this if you want something fancier 🙂


Suggested change

// Setup our simple loading screen, it uses an image as spinner and a loading text bellow it.

// INFO: you need to change this if you want something fancier 🙂

/// Setup our simple loading screen, it uses an image as spinner and a loading text below it.

///

/// INFO: you need to change this if you want something fancier 🙂

Double space is newline in generated docs, and it is appropriate to keep the first paragraph short.

Nilirad · 2022-05-16T16:12:13Z

examples/asset/loading_screen.rs

+    // System that listens for our `AssetEvent` and changes our game state after we finished loading our asset
+    // INFO: you need to change this if you have different assets 🙂


Suggested change

// System that listens for our `AssetEvent` and changes our game state after we finished loading our asset

// INFO: you need to change this if you have different assets 🙂

/// System that listens for our `AssetEvent` and changes our game state after we finished loading our asset.

///

/// INFO: you need to change this if you have different assets 🙂

Nilirad · 2022-05-16T16:12:23Z

examples/asset/loading_screen.rs

+        }
+    }
+
+    // Animates aka rotates the bevy bird `UiIamge` clockwise.


Suggested change

// Animates aka rotates the bevy bird `UiIamge` clockwise.

/// Animates aka rotates the bevy bird `UiImage` clockwise.

Nilirad · 2022-05-16T16:12:36Z

examples/asset/loading_screen.rs

+        }
+    }
+
+    // Close our loading screen. This system is called when we exit `LoadingState::Loading`.


Suggested change

// Close our loading screen. This system is called when we exit `LoadingState::Loading`.

/// Close our loading screen. This system is called when we exit `LoadingState::Loading`.

Nilirad · 2022-05-16T16:32:48Z

examples/asset/loading_screen.rs

+                // we don't care about these events in our example
+                AssetEvent::Modified { handle: _ } => {}
+                AssetEvent::Removed { handle: _ } => {}


Suggested change

// we don't care about these events in our example

AssetEvent::Modified { handle: _ } => {}

AssetEvent::Removed { handle: _ } => {}

_ => {}

This is a much cleaner solution, and it implicitly transmits the “I don't care about the other cases” message.

Nilirad · 2022-05-16T16:46:25Z

examples/asset/loading_screen.rs

+use bevy::prelude::*;
+
+/// This example illustrates, how a loading screen can be implemented. It has an animated spinner and listens for an `AssetEvent`.


Suggested change

use bevy::prelude::*;

/// This example illustrates, how a loading screen can be implemented. It has an animated spinner and listens for an `AssetEvent`.

//! Illustrates how a loading screen can be implemented.

//!

//! The loading screen logic is handled by the [`SimpleLoadingScreenPlugin`].

//! Its functionality is based on the states defined by [`LoadingState`]. When

//! the example starts, the initial state is `LoadingState::Loading`. Then the

//! `setup_game` startup system requests that an asset gets loaded. While the

//! loading screen runs, the `asset_listening_system` checks if the asset has been

//! fully loaded. When this happens, the state is changed to `LoadingState::Done`.

//! This will remove the loading screen and make `game_system` run.

//!

//! [`SimpleLoadingScreenPlugin`]: plugin::SimpleLoadingScreenPlugin

//! [`LoadingState`]: plugin::LoadingState

use bevy::prelude::*;

I changed the doc comment to module-level, as it is the new standard. I also added a more comprehensive description.

bas-ie · 2024-10-02T00:04:23Z

Now superseded by the likes of bevy_new_2d.

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 9, 2022

alice-i-cecile added C-Examples An addition or correction to our examples and removed S-Needs-Triage This issue needs to be labelled labels Jan 9, 2022

Weasy666 force-pushed the loading_screen branch 2 times, most recently from e5c931b to ac97e76 Compare January 11, 2022 08:10

IceSentry reviewed Jan 11, 2022

View reviewed changes

Weasy666 force-pushed the loading_screen branch from 6f3cf91 to f874a0e Compare January 12, 2022 08:21

IceSentry reviewed Jan 12, 2022

View reviewed changes

examples/asset/loading_screen.rs Outdated Show resolved Hide resolved

Weasy666 commented Jan 12, 2022

View reviewed changes

examples/asset/loading_screen.rs Outdated Show resolved Hide resolved

alice-i-cecile approved these changes Apr 18, 2022

View reviewed changes

bors bot added a commit that referenced this pull request Apr 18, 2022

Try #3618:

eed6940

alice-i-cecile added the A-Assets Load files from disk to use for things like images, models, and sounds label Apr 26, 2022

Weasy666 added 4 commits May 16, 2022 09:49

Add loading_screen example

822246f

Move loading screen into a Plugin and a plugin module, to make the se…

cd0d9e2

…paration more clear.

Appease the CI gods

32f0edb

Run cargo fmt

30af1ac

Weasy666 added 3 commits May 16, 2022 09:50

Address review comments

e02c88a

Appease clippy

055b2ff

Move asset_listener and State into Plugin and remove loading text man…

73ab064

…ipulation, because `loading_screen` query does not work because of system ordering.

Weasy666 force-pushed the loading_screen branch from d85d6c6 to 73ab064 Compare May 16, 2022 07:51

Nilirad suggested changes May 16, 2022

View reviewed changes

bas-ie closed this Oct 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add loading_screen example #3618

Add loading_screen example #3618

Weasy666 commented Jan 9, 2022

alice-i-cecile commented Jan 9, 2022

Weasy666 commented Jan 10, 2022

alice-i-cecile commented Jan 10, 2022

Weasy666 commented Jan 10, 2022

alice-i-cecile commented Jan 11, 2022

Weasy666 commented Jan 11, 2022

IceSentry Jan 11, 2022

IceSentry Jan 11, 2022

Weasy666 Jan 12, 2022

IceSentry Jan 12, 2022

Weasy666 Jan 12, 2022

alice-i-cecile commented Jan 11, 2022

alice-i-cecile commented Apr 18, 2022

alice-i-cecile commented Apr 18, 2022

IceSentry commented Apr 18, 2022

bors bot commented Apr 18, 2022

Weasy666 commented Apr 25, 2022

IceSentry commented Apr 25, 2022

Nilirad commented May 3, 2022

alice-i-cecile commented May 16, 2022

Nilirad left a comment

Nilirad May 16, 2022

Weasy666 May 25, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

Nilirad May 16, 2022

bas-ie commented Oct 2, 2024

	SystemSet::on_update(self.loading_state.clone()).with_system(animate_spinner),
	)
	.add_system_set(
	SystemSet::on_exit(self.loading_state.clone())

	// Here we prepare our gaming scene, meaning we setup our ingame camera and load our asset.
	/// Here we prepare our gaming scene, meaning we setup our ingame camera and load our asset.

	// Your, or at least one of your, game systems.
	/// Your, or at least one of your, game systems.

		// Setup our simple loading screen, it uses an image as spinner and a loading text bellow it.
		// INFO: you need to change this if you want something fancier 🙂

		// System that listens for our `AssetEvent` and changes our game state after we finished loading our asset
		// INFO: you need to change this if you have different assets 🙂

	// Animates aka rotates the bevy bird `UiIamge` clockwise.
	/// Animates aka rotates the bevy bird `UiImage` clockwise.

	// Close our loading screen. This system is called when we exit `LoadingState::Loading`.
	/// Close our loading screen. This system is called when we exit `LoadingState::Loading`.

		use bevy::prelude::*;

		/// This example illustrates, how a loading screen can be implemented. It has an animated spinner and listens for an `AssetEvent`.

-use bevy::prelude::*;
-/// This example illustrates, how a loading screen can be implemented. It has an animated spinner and listens for an `AssetEvent`.
+//! Illustrates how a loading screen can be implemented.
+//!
+//! The loading screen logic is handled by the [`SimpleLoadingScreenPlugin`].
+//! Its functionality is based on the states defined by [`LoadingState`]. When
+//! the example starts, the initial state is `LoadingState::Loading`. Then the
+//! `setup_game` startup system requests that an asset gets loaded. While the
+//! loading screen runs, the `asset_listening_system` checks if the asset has been
+//! fully loaded. When this happens, the state is changed to `LoadingState::Done`.
+//! This will remove the loading screen and make `game_system` run.
+//!
+//! [`SimpleLoadingScreenPlugin`]: plugin::SimpleLoadingScreenPlugin
+//! [`LoadingState`]: plugin::LoadingState
+use bevy::prelude::*;

Add loading_screen example #3618

Add loading_screen example #3618

Conversation

Weasy666 commented Jan 9, 2022

Objective

Solution

alice-i-cecile commented Jan 9, 2022

Weasy666 commented Jan 10, 2022

alice-i-cecile commented Jan 10, 2022

Weasy666 commented Jan 10, 2022

alice-i-cecile commented Jan 11, 2022

Weasy666 commented Jan 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile commented Jan 11, 2022

alice-i-cecile commented Apr 18, 2022

alice-i-cecile commented Apr 18, 2022

IceSentry commented Apr 18, 2022

bors bot commented Apr 18, 2022

try

Weasy666 commented Apr 25, 2022

IceSentry commented Apr 25, 2022

Nilirad commented May 3, 2022

alice-i-cecile commented May 16, 2022

Nilirad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bas-ie commented Oct 2, 2024