Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod info;
mod io;
mod loader;
mod path;
mod ser;

pub mod prelude {
#[doc(hidden)]
Expand All @@ -25,6 +26,7 @@ pub use info::*;
pub use io::*;
pub use loader::*;
pub use path::*;
pub use ser::*;

use bevy_app::{prelude::Plugin, AppBuilder};
use bevy_ecs::{
Expand Down
90 changes: 90 additions & 0 deletions crates/bevy_asset/src/ser.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
use std::cell::Cell;

use bevy_reflect::Uuid;
use serde::{Deserialize, Deserializer, Serialize, Serializer};

use crate::{Asset, AssetServer, Handle, HandleId, HandleUntyped};

///////////////////////////////////////////////////////////////////////////////
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///////////////////////////////////////////////////////////////////////////////


thread_local! {
static ASSET_SERVER: Cell<Option<AssetServer>> = Cell::new(None);
}

#[derive(Serialize, Deserialize)]
enum AssetRef {
Default,
/// Used for static handles like the `PBR_PIPELINE_HANDLE` or a embedded assets
Local(Uuid, u64),
/// Loads from a file
External(String),
}

impl<T: Asset> Serialize for Handle<T> {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let path = ASSET_SERVER
.with(|cell| {
let server = cell.replace(None);
let path = server.as_ref().and_then(|server| {
// TODO: `get_handle_path` does absolutely nothing issue #1290
Copy link
Member

Choose a reason for hiding this comment

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

This has been resolved.

server.get_handle_path(self).map(|asset_path| {
let mut path = asset_path.path().to_string_lossy().to_string();
if let Some(label) = asset_path.label() {
path.push('#');
path.push_str(label);
}
AssetRef::External(path)
})
});
cell.replace(server);
path
})
.unwrap_or_else(|| match &self.id {
HandleId::Id(type_uuid, id) => AssetRef::Local(*type_uuid, *id),
HandleId::AssetPathId(_) => AssetRef::Default,
});

path.serialize(serializer)
}
}

impl<'de, T: Asset> Deserialize<'de> for Handle<T> {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
match AssetRef::deserialize(deserializer)? {
AssetRef::Default => Ok(Handle::default()),
AssetRef::Local(type_uuid, id) => {
Ok(HandleUntyped::weak_from_u64(type_uuid, id).typed())
}
AssetRef::External(path) => ASSET_SERVER.with(|cell| {
let server = cell.replace(None);
let handle = server
.as_ref()
.map(|server| server.load(path.as_str()))
.unwrap_or_default();
cell.replace(server);
Ok(handle)
}),
}
}
}

impl AssetServer {
/// Enables asset references to be serialized or deserialized
pub fn with_asset_refs_serialization<F, T>(&self, f: F) -> T
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you give f a more descriptive name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

f, func are commonly used in this situations, anything in mind?

Copy link
Contributor

@NathanSWard NathanSWard Jun 7, 2021

Choose a reason for hiding this comment

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

Something like serde_func (that expresses the functions does some sort of (de)serialization) would be nice.

where
F: FnOnce() -> T,
{
ASSET_SERVER.with(|key| {
key.replace(Some(self.clone()));
let result = (f)();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have an example use case of how this API is better than the previous one?
It worries me here that T is not constrained on Serialize or Deserialize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the last API didn't allow to de::DeserializeSeed to be used for stateful de-serialization

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that makes sense.
However, I'm still not a big fan of just having the parameter be f: impl FnOnce() -> T.
you could give this any f that returns any type, which as a user of the function, isn't very informative.
If we could come up with some way to constrain T to better express that T needs to be serializable/deserializable via the AssetServer context, that would be best IMO.

key.replace(None);
result
})
}
}