Don't auto create assets folder

[nvdaz]

Objective

• Don't automatically create an assets folder
• resolves AssetPlugin always creates empty assets directory #11208

Solution

• Removes directory creation from file reader.
• Clearer panic when using file watcher and asset folder doesn't exist

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[alice-i-cecile]

Should we document that FileAssetReader::new does not create a directory?

I don't feel particularly strongly, but we should fix the typo pointed out above.

[nvdaz]

I feel it's probably unnecessary since I personally wouldn't expect a file reader to do create anything. It is much more surprising as it is right now IMO–creating a directory undocumented.

[alice-i-cecile]

@thepackett, could you review this little PR? Does this make sense with your understanding of the asset lifecycle?

[thepackett]

Honestly I can't say I support this change for a couple reasons.

I think creating the asset directory is the appropriate default behavior, since if it didn't create the asset directory automatically then you would manually have to create the folder and work your way through any possible errors or misunderstandings along the way.

Secondly, this behavior can be disabled. When the AssetPlugin starts it checks to see if any AssetSourceBuilders are in the AssetSourceBuilders resource, and if there aren't any then it uses the default AssetSources, which includes the file source. Thus you can disable this by providing your own AssetSourceBuilders before adding the Asset Plugin as per this example.

As for the documentation and panic error message, this is good and I will always approve of better error messages and documentation.

[nvdaz]

I see what you're saying, and I might even agree if FileAssetReader wasn't the default, that way creating the folder would be explicitly opt-in. But it's not and I think the current behavior is really unexpected for those who aren't aware that Bevy automatically does this. I discovered this because there was an empty folder in my project directory, so I naturally tried deleting it to no avail... I originally hadn't even suspected Bevy because in my head, a library doesn't update the file system unless I explicitly ask.

In my opinion, automatically creating an assets folder without context isn't very helpful either. It wouldn't be obvious to a new user why the folder exists or how to import assets just from it existing. They would still have to discover AssetServer::load, which helpfully tells the user the full asset path searched for if not found.

And just to clarify since I think it might be unclear: this doesn't cause a panic with just the default features and no assets folder. Bevy runs just fine without an assets folder. The updated panic message only happens if the user enables the file_watcher feature, so it would be pretty odd if they did this without having an assets folder (though the panic would tell them it was caused by this feature and the assets folder not being found).

I would definitely agree, though, that better documentation would be useful. I just feel it wouldn't be very helpful here as beginners would probably not look at the internals. There should probably be a section on asset loading in The Bevy Book. I could add a comment here though, if required, just feels a little superfluous to say that a file reader does not create a directory.

[AxiomaticSemantics]

I'm a vote for the panic behaviour as is. A potential follow-up could be having bevy_assets use a build.rs and allowing to opt out. Further out in the future a cargo bevy-project generator perhaps.

[nicopap]

I'd prefer if bevy did not silently create file directories, so I'm in favor of merging this.

[thepackett]

I know I'm somewhat repeating myself here, but I don't believe that the defaults for plugins are unreasonable. Bevy is built to be as modular and as flexible as possible so that you can really dig into the engine and modify and configure it however you want. However, that flexibility comes at the price of complexity for the developer. The purpose of the default behavior is to provide a decent starting point for the majority of users so they can get up and running as quickly as possible without having to spend hours configuring every plugin they use in every project they create. I think that the AssetPlugin using the File AssetSource by default is reasonable, and creating the directory that all assets must be placed in for the File AssetSource to work by default is also reasonable.

[nvdaz]

While I acknowledge the reasoning behind creating an empty assets folder being the default, I'd contend that it extends beyond what one might reasonably expect, especially when Bevy is installed as a dependency via Cargo (since it does not own the project and thus shouldn't make changes imo). Such behavior might be more fitting for a standalone Bevy editor (i.e. like Unreal/Unity), where ownership is clear.

I think my previous comment made my perspective clear beyond that. It ultimately boils down to weighing the cost of silently creating the assets folder against the convenience of it always being there by default.

[thepackett]

"I think my previous comment made my perspective clear beyond that. It ultimately boils down to weighing the cost of silently creating the assets folder against the convenience of it always being there by default."

Agreed. We've both made our case and it really comes down to the core vision for bevy, which is probably why this has been tagged as controversial. This means that Cart (or two SMEs, which I don't believe we have for Bevy Asset) needs to take a look at this to decide.

[nvdaz]

Yep. I interpreted the behavior as a bug since it didn't happen in 0.11 or previous and the change wasn't documented. Hadn't intended to comment on Bevy's philosophy, but I guess that's where this wound up!

In case it's decided that this should be the new default, I'd like to see a warning because it's still unexpected.

It would be great if @cart could comment, especially since the behavior was added in 5eb292d.

[mockersf]

I would be for not creating the asset folder. This should be created by the project template / editor, or by the user explicitly

[cart]

I would be for not creating the asset folder. This should be created by the project template / editor, or by the user explicitly

Agreed!