From dac4a5bbb4336527ffd0880233b92f5bc4f93c54 Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Tue, 10 Sep 2024 02:01:14 +1000 Subject: [PATCH] Depreciate `LoadAndSave` Asset Processor (#15090) # Objective - Fixes #15060 ## Solution - Added `IdentityAssetTransformer` which is an `AssetTransformer` which infallibly returns the input `Asset` unmodified. - Replaced `LoadAndSave` and `LoadAndSaveSettings` with type definitions linking back to `LoadTransformAndSave` and `LoadTransformAndSaveSettings` respectively. - Marked `LoadAndSave` and `LoadAndSaveSettings` as depreciated with a migration guide included, hinting to the user to use the underlying type instead. ## Testing - Ran CI locally --- ## Migration Guide - Replace `LoadAndSave` with `LoadTransformAndSave::Asset>, S>` - Replace `LoadAndSaveSettings` with `LoadTransformAndSaveSettings` --- crates/bevy_asset/src/processor/process.rs | 74 +++++++--------------- crates/bevy_asset/src/transformer.rs | 36 +++++++++++ crates/bevy_render/src/texture/mod.rs | 15 +++-- 3 files changed, 70 insertions(+), 55 deletions(-) diff --git a/crates/bevy_asset/src/processor/process.rs b/crates/bevy_asset/src/processor/process.rs index 349c047005586..92a13f21ff44e 100644 --- a/crates/bevy_asset/src/processor/process.rs +++ b/crates/bevy_asset/src/processor/process.rs @@ -1,4 +1,5 @@ use crate::io::SliceReader; +use crate::transformer::IdentityAssetTransformer; use crate::{ io::{ AssetReaderError, AssetWriterError, MissingAssetWriterError, @@ -47,6 +48,11 @@ pub trait Process: Send + Sync + Sized + 'static { /// an [`AssetSaver`] that allows you save any `S` asset. However you can /// also implement [`Process`] directly if [`LoadTransformAndSave`] feels limiting or unnecessary. /// +/// If your [`Process`] does not need to transform the [`Asset`], you can use [`IdentityAssetTransformer`] as `T`. +/// This will directly return the input [`Asset`], allowing your [`Process`] to directly load and then save an [`Asset`]. +/// However, this pattern should only be used for cases such as file format conversion. +/// Otherwise, consider refactoring your [`AssetLoader`] and [`AssetSaver`] to isolate the transformation step into an explicit [`AssetTransformer`]. +/// /// This uses [`LoadTransformAndSaveSettings`] to configure the processor. /// /// [`Asset`]: crate::Asset @@ -60,6 +66,18 @@ pub struct LoadTransformAndSave< marker: PhantomData L>, } +impl> From + for LoadTransformAndSave, S> +{ + fn from(value: S) -> Self { + LoadTransformAndSave { + transformer: IdentityAssetTransformer::new(), + saver: value, + marker: PhantomData, + } + } +} + /// Settings for the [`LoadTransformAndSave`] [`Process::Settings`] implementation. /// /// `LoaderSettings` corresponds to [`AssetLoader::Settings`], `TransformerSettings` corresponds to [`AssetTransformer::Settings`], @@ -98,30 +116,16 @@ impl< /// This uses [`LoadAndSaveSettings`] to configure the processor. /// /// [`Asset`]: crate::Asset -pub struct LoadAndSave> { - saver: S, - marker: PhantomData L>, -} - -impl> From for LoadAndSave { - fn from(value: S) -> Self { - LoadAndSave { - saver: value, - marker: PhantomData, - } - } -} +#[deprecated = "Use `LoadTransformAndSave::Asset>, S>` instead"] +pub type LoadAndSave = + LoadTransformAndSave::Asset>, S>; /// Settings for the [`LoadAndSave`] [`Process::Settings`] implementation. /// /// `LoaderSettings` corresponds to [`AssetLoader::Settings`] and `SaverSettings` corresponds to [`AssetSaver::Settings`]. -#[derive(Serialize, Deserialize, Default)] -pub struct LoadAndSaveSettings { - /// The [`AssetLoader::Settings`] for [`LoadAndSave`]. - pub loader_settings: LoaderSettings, - /// The [`AssetSaver::Settings`] for [`LoadAndSave`]. - pub saver_settings: SaverSettings, -} +#[deprecated = "Use `LoadTransformAndSaveSettings` instead"] +pub type LoadAndSaveSettings = + LoadTransformAndSaveSettings; /// An error that is encountered during [`Process::process`]. #[derive(Error, Debug)] @@ -213,36 +217,6 @@ where } } -impl> Process - for LoadAndSave -{ - type Settings = LoadAndSaveSettings; - type OutputLoader = Saver::OutputLoader; - - async fn process<'a>( - &'a self, - context: &'a mut ProcessContext<'_>, - meta: AssetMeta<(), Self>, - writer: &'a mut Writer, - ) -> Result<::Settings, ProcessError> { - let AssetAction::Process { settings, .. } = meta.asset else { - return Err(ProcessError::WrongMetaType); - }; - let loader_meta = AssetMeta::::new(AssetAction::Load { - loader: std::any::type_name::().to_string(), - settings: settings.loader_settings, - }); - let loaded_asset = context.load_source_asset(loader_meta).await?; - let saved_asset = SavedAsset::::from_loaded(&loaded_asset).unwrap(); - let output_settings = self - .saver - .save(writer, saved_asset, &settings.saver_settings) - .await - .map_err(|error| ProcessError::AssetSaveError(error.into()))?; - Ok(output_settings) - } -} - /// A type-erased variant of [`Process`] that enables interacting with processor implementations without knowing /// their type. pub trait ErasedProcessor: Send + Sync { diff --git a/crates/bevy_asset/src/transformer.rs b/crates/bevy_asset/src/transformer.rs index 1b2cd92991c15..21abb3331befc 100644 --- a/crates/bevy_asset/src/transformer.rs +++ b/crates/bevy_asset/src/transformer.rs @@ -4,7 +4,9 @@ use bevy_utils::{ConditionalSendFuture, HashMap}; use serde::{Deserialize, Serialize}; use std::{ borrow::Borrow, + convert::Infallible, hash::Hash, + marker::PhantomData, ops::{Deref, DerefMut}, }; @@ -241,3 +243,37 @@ impl<'a, A: Asset> TransformedSubAsset<'a, A> { self.labeled_assets.keys().map(|s| &**s) } } + +/// An identity [`AssetTransformer`] which infallibly returns the input [`Asset`] on transformation.] +pub struct IdentityAssetTransformer { + _phantom: PhantomData A>, +} + +impl IdentityAssetTransformer { + pub const fn new() -> Self { + Self { + _phantom: PhantomData, + } + } +} + +impl Default for IdentityAssetTransformer { + fn default() -> Self { + Self::new() + } +} + +impl AssetTransformer for IdentityAssetTransformer { + type AssetInput = A; + type AssetOutput = A; + type Settings = (); + type Error = Infallible; + + async fn transform<'a>( + &'a self, + asset: TransformedAsset, + _settings: &'a Self::Settings, + ) -> Result, Self::Error> { + Ok(asset) + } +} diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index 9b1a592773409..566579d7c86e3 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/crates/bevy_render/src/texture/mod.rs @@ -107,11 +107,16 @@ impl Plugin for ImagePlugin { .world() .get_resource::() { - processor.register_processor::>( - CompressedImageSaver.into(), - ); - processor - .set_default_processor::>("png"); + processor.register_processor::, + CompressedImageSaver, + >>(CompressedImageSaver.into()); + processor.set_default_processor::, + CompressedImageSaver, + >>("png"); } if let Some(render_app) = app.get_sub_app_mut(RenderApp) {