-
Notifications
You must be signed in to change notification settings - Fork 82
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat!: redesign LdtkProject with better level data accessors and corr…
…ect modeling of internal/external levels (#244) Closes #205 This is the final PR for redesigning the asset types. The main improvements of this redesign are that the type provides better APIs for accessing raw or loaded level data, and internal/external levels are modeled more correctly. The `LdtkProject` type is still the asset type and stores most metadata applicable to either level locale, along with `LdtkProjectData`. `LdtkProjectData` is a enum whose variants store the actual ldtk json data, with a variant for each level locality. The internal types of the variants are `LdtkJsonWithMetadata<L: LevelLocale>`, with either `InternalLevels` or `ExternalLevels` as `L`. `LdtkJsonWithMetadata<L>` is a generic type storing the actual ldtk json data + level metadata, with either `InternalLevels` or `ExternalLevels` as `L`. Splitting these up like this allows us to define some methods that are exclusive to each level locality. This is important because accessing loaded level data is a very different operation memory-wise for each case (indexing the `LdtkJson` for internal-levels, or accessing the `Assets<LdtkLevel>` asset store for external levels). But other methods can be provided for either case, either with a generic implementation at the lowest level, or exposing transparent implementations in the higher-level types. An important point about this new design is that `LdtkLevel` assets are no longer used in the internal-levels case. Level entities will only ever have a `LevelIid` component, no longer a `Handle<LdtkLevel>`. The handle for the asset in the external-levels case is only stored inside the `LdtkProject`. To help make this change clear, `LdtkLevel` has been renamed `LdtkExternalLevel`. Doing things this way actually fixes an egregious clone of all level data that is in all previous versions of this plugin. See the documentation of `LdtkProject` to learn about the best ways to access loaded level data now. feat!: LdtkLevel renamed to LdtkExternalLevel and is no longer used as a component (#244)
- Loading branch information
Showing
14 changed files
with
1,178 additions
and
773 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| use crate::ldtk::{loaded_level::LoadedLevel, Level}; | ||
| use bevy::{ | ||
| asset::{AssetLoader, LoadContext, LoadedAsset}, | ||
| prelude::*, | ||
| reflect::TypeUuid, | ||
| utils::BoxedFuture, | ||
| }; | ||
| use thiserror::Error; | ||
|
|
||
| /// Secondary asset for loading external-levels ldtk files, specific to level data. | ||
| /// | ||
| /// Loaded as a dependency of the [`LdtkProject`] asset. | ||
| /// | ||
| /// Requires the `external_levels` feature to be enabled. | ||
| /// | ||
| /// [`LdtkProject`]: crate::assets::LdtkProject | ||
| #[derive(Clone, Debug, PartialEq, TypeUuid, Reflect)] | ||
| #[uuid = "5448469b-2134-44f5-a86c-a7b829f70a0c"] | ||
| pub struct LdtkExternalLevel { | ||
| /// Raw LDtk level data. | ||
| data: Level, | ||
| } | ||
|
|
||
| impl LdtkExternalLevel { | ||
| /// Construct a new [`LdtkExternalLevel`]. | ||
| /// | ||
| /// Only available for testing. | ||
| /// This type should only be constructed via the bevy asset system under normal use. | ||
| #[cfg(test)] | ||
| pub fn new(data: Level) -> LdtkExternalLevel { | ||
| LdtkExternalLevel { data } | ||
| } | ||
|
|
||
| /// Internal LDtk level data as a [`LoadedLevel`]. | ||
| pub fn data(&self) -> LoadedLevel { | ||
| LoadedLevel::try_from(&self.data) | ||
| .expect("construction of LdtkExternalLevel should guarantee that the level is loaded.") | ||
| } | ||
| } | ||
|
|
||
| /// Errors that can occur when loading an [`LdtkExternalLevel`] asset. | ||
| #[derive(Debug, Error)] | ||
| pub enum LdtkExternalLevelLoaderError { | ||
| /// External LDtk level should contain all level data, but some level has null layers. | ||
| #[error("external LDtk level should contain all level data, but some level has null layers")] | ||
| NullLayers, | ||
| } | ||
|
|
||
| /// AssetLoader for [`LdtkExternalLevel`] | ||
| #[derive(Default)] | ||
| pub struct LdtkExternalLevelLoader; | ||
|
|
||
| impl AssetLoader for LdtkExternalLevelLoader { | ||
| fn load<'a>( | ||
| &'a self, | ||
| bytes: &'a [u8], | ||
| load_context: &'a mut LoadContext, | ||
| ) -> BoxedFuture<'a, anyhow::Result<()>> { | ||
| Box::pin(async move { | ||
| let data: Level = serde_json::from_slice(bytes)?; | ||
|
|
||
| if data.layer_instances.is_none() { | ||
| Err(LdtkExternalLevelLoaderError::NullLayers)?; | ||
| } | ||
|
|
||
| let ldtk_level = LdtkExternalLevel { data }; | ||
|
|
||
| let loaded_asset = LoadedAsset::new(ldtk_level); | ||
|
|
||
| load_context.set_default_asset(loaded_asset); | ||
| Ok(()) | ||
| }) | ||
| } | ||
|
|
||
| fn extensions(&self) -> &[&str] { | ||
| &["ldtkl"] | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use fake::{Fake, Faker}; | ||
|
|
||
| use crate::ldtk::fake::UnloadedLevelFaker; | ||
|
|
||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn data_accessor_for_loaded_level_succeeds() { | ||
| // default level faker creates a loaded level | ||
| let level: Level = Faker.fake(); | ||
|
|
||
| let ldtk_external_level = LdtkExternalLevel::new(level.clone()); | ||
|
|
||
| assert_eq!(ldtk_external_level.data().raw(), &level); | ||
| } | ||
|
|
||
| #[test] | ||
| #[should_panic] | ||
| fn data_accessor_for_unloaded_level_panics() { | ||
| let level: Level = UnloadedLevelFaker.fake(); | ||
|
|
||
| let ldtk_external_level = LdtkExternalLevel::new(level.clone()); | ||
|
|
||
| let _should_panic = ldtk_external_level.data(); | ||
| } | ||
| } |
Oops, something went wrong.