-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] Add an option to disable spawning of some/all IntGrid entities #272
Comments
This is reasonable. I think a simple |
focustense
added a commit
to focustense/bevy_ecs_ldtk
that referenced
this issue
Dec 3, 2023
focustense
added a commit
to focustense/bevy_ecs_ldtk
that referenced
this issue
Dec 4, 2023
focustense
added a commit
to focustense/bevy_ecs_ldtk
that referenced
this issue
Dec 5, 2023
Trouv
pushed a commit
that referenced
this issue
Dec 6, 2023
… identifier (#275) I made this a separate `LdtkExclusions` struct because, even though there's only one use case for this now, it would be a logical extension to add other exclusions in the future (like entities, both IID and UID based), and if so, it's better to group them together instead of cluttering up the main `LdtkSettings`. Caveat: Adding a `Vec` (or any other collection) to the `LdtkSettings` or any sub-structure means we can no longer derive `Copy`. It doesn't appear to be required by any part of `bevy_ecs_ldtk` itself, and there's no compelling reason why users should absolutely need it (copying a `Resource` is pretty rare?), but this is potentially a breaking change for some users. The only way around this would be to avoid putting this in `LdtkSettings` altogether and define the `LdtkExclusions` as its own resource (and modify the various systems accordingly). I feel that's a YAGNI problem so I went the easy route and just removed `Copy`. Unit testing seems infeasible at the moment due to the complexity of the `spawn_level` method and absence of existing test infra for it, but I manually tested the exact same change on the 0.8 branch in an actual game and it worked perfectly. Note also that the use of `Vec` over `HashSet` is intentional. I'd expect there to be a very small number of items in that list (say 2 or 3). In lists with fewer than 10-15 items, the cost of the hash tends to exceed the cost of the iteration ([example](https://gist.github.com/daboross/976978d8200caf86e02acb6805961195)). Fixes #272
Trouv
added a commit
that referenced
this issue
Feb 11, 2024
🤖 I have created a release *beep* *boop* --- ## [0.9.0](v0.8.0...v0.9.0) (2024-02-11) ### ⚠ BREAKING CHANGES * upgrade to bevy 0.12 ([#265](#265)) * upgrade to LDtk 1.5.3, dropping support for previous versions ([#295](#295)) * add `SpawnExclusions` to `LdtkSettings` for skipping layers by identifier ([#275](#275)) * add layer entity for Entity layers, changing the hierarchy ([#257](#257)) * upgrade to LDtk types and examples to 1.4.1 (drop support for <1.4.1) ([#256](#256)) * LdtkLevel renamed to LdtkExternalLevel and is no longer used as a component ([#244](#244)) * redesign LdtkProject with better level data accessors and correct modeling of internal/external levels ([#244](#244)) * use the bundle's `Default` implementation rather than the field's in `LdtkEntity` and `LdtkIntCell` derive macros ([#222](#222)) * add `RawLevelAccessor` trait for `LdtkJson` level borrowing/iteration, replacing existing methods ([#225](#225)) * add `LevelIndices` type defining a level's location in a project and use it in `LevelSelection::Indices` ([#221](#221)) * change `LevelEvent` inner types from `String` to `LevelIid` ([#219](#219)) * change `LevelSet` inner type from `String` to `LevelIid` ([#219](#219)) * change `LevelSelection::Iid` inner type from `String` to `LevelIid` ([#219](#219)) * replace `LevelSet::from_iid` with `LevelSet::from_iids`, which can convert from any collection of strings. ([#219](#219)) * use new LevelIid type in LevelEvent, LevelSet, and LevelSelection, plus other improvements ([#219](#219)) * `LdtkProject::project` and `LdtkLevel::level` fields have both been renamed to `data` ([#206](#206)) * All fields of `LdtkProject` and `LdtkLevel` are now privatized, and have immutable getter methods ([#206](#206)) * `LevelMap` and `TilesetMap` type aliases have been removed ([#206](#206)) * `LdtkAsset` and `LdtkProject` are now exported in new `assets` module instead of `lib.rs` ([#206](#206)) * asset `Loader` types are now private ([#206](#206)) * `LdtkAsset` renamed to `LdtkProject` ([#206](#206)) ### Features * add `LevelIndices` type defining a level's location in a project and use it in `LevelSelection::Indices` ([#221](#221)) ([59618fe](59618fe)) * add `RawLevelAccessor` trait for `LdtkJson` level borrowing/iteration, replacing existing methods ([#225](#225)) ([d3de2d9](d3de2d9)) * add `SpawnExclusions` to `LdtkSettings` for skipping layers by identifier ([#275](#275)) ([282404d](282404d)), closes [#272](#272) * add layer entity for Entity layers, changing the hierarchy ([#257](#257)) ([ee20a53](ee20a53)) * add LdtkJsonWithMetadata type for representing internal- and external-level project data with generics ([#242](#242)) ([630434a](630434a)) * add LdtkProjectData for representing either internal- or external-level project data concretely ([#243](#243)) ([c530bc9](c530bc9)) * add level locale types and begin splitting internal_levels and external_levels features ([#237](#237)) ([8129e55](8129e55)) * add LevelIid component and spawn it on every level ([#215](#215)) ([ad83455](ad83455)) * add LoadedLevel type that wraps around levels with complete data ([#214](#214)) ([3d40c15](3d40c15)) * add types and traits around LevelMetadata ([#229](#229)) ([382dea2](382dea2)) * change `LevelEvent` inner types from `String` to `LevelIid` ([#219](#219)) ([0039ed7](0039ed7)) * change `LevelSelection::Iid` inner type from `String` to `LevelIid` ([#219](#219)) ([0039ed7](0039ed7)) * change `LevelSet` inner type from `String` to `LevelIid` ([#219](#219)) ([0039ed7](0039ed7)) * LdtkLevel renamed to LdtkExternalLevel and is no longer used as a component ([#244](#244)) ([670cd4e](670cd4e)) * redesign LdtkProject with better level data accessors and correct modeling of internal/external levels ([#244](#244)) ([670cd4e](670cd4e)) * replace `LevelSet::from_iid` with `LevelSet::from_iids`, which can convert from any collection of strings. ([#219](#219)) ([0039ed7](0039ed7)) * upgrade to bevy 0.12 ([#265](#265)) ([194731e](194731e)) * upgrade to LDtk 1.5.3, dropping support for previous versions ([#295](#295)) ([4926a50](4926a50)) * upgrade to LDtk types and examples to 1.4.1 (drop support for <1.4.1) ([#256](#256)) ([ab21e2c](ab21e2c)) * use new LevelIid type in LevelEvent, LevelSet, and LevelSelection, plus other improvements ([#219](#219)) ([0039ed7](0039ed7)) * use the bundle's `Default` implementation rather than the field's in `LdtkEntity` and `LdtkIntCell` derive macros ([#222](#222)) ([f003127](f003127)) ### Bug Fixes * don't apply level set until project and dependencies are completely loaded ([#296](#296)) ([dbfe1c6](dbfe1c6)) * normalize resolved asset paths using `path_clean` ([#255](#255)) ([33a8998](33a8998)), closes [#240](#240) * only spawn invisible tiles on first sub-layer of AutoTile+IntGrid layers ([#231](#231)) ([d2873e3](d2873e3)) * recalculate layer offset to adjust for tileset sizes ([#254](#254)) ([c00085d](c00085d)) * use entity definition tile size instead of entity instance tile size as basis when calculating ldtk entity scale ([#271](#271)) ([833af01](833af01)) ### Documentation Changes * add 0.8 to 0.9 migration guide ([#266](#266)) ([bb91660](bb91660)) * add collectathon cargo example ([#288](#288)) ([32dfb85](32dfb85)) * add mdbook with outline and introduction ([#261](#261)) ([810b25a](810b25a)) * add tile-based game example w/ a tutorial in the book, replacing getting-started guide ([#269](#269)) ([2d43efa](2d43efa)) * document all-features in docs.rs ([#252](#252)) ([321bb07](321bb07)) * reference book in API ref and README.md, replacing redundant sections ([#282](#282)) ([e7afdad](e7afdad)) * remove README.md caveat for hot reloading external levels ([#253](#253)) ([59eb6b3](59eb6b3)) * write *Anatomy of the World* chapter of book ([#285](#285)) ([29d5e33](29d5e33)) * write *Create bevy relations from ldtk entity references* chapter of book ([#287](#287)) ([8080f24](8080f24)) * write *Game Logic Integration* chapter of the book ([#279](#279)) ([a62a556](a62a556)) * write *Level Selection* chapter of book ([#284](#284)) ([226c60c](226c60c)) * write *Make level selection follow player* chapter of book ([#293](#293)) ([201d908](201d908)) * write *Respawn levels and worlds* chapter of book ([#289](#289)) ([55ed30f](55ed30f)) ### Code Refactors * `LdtkAsset` and `LdtkProject` are now exported in new `assets` module instead of `lib.rs` ([#206](#206)) ([fe44774](fe44774)) * `LdtkAsset` renamed to `LdtkProject` ([#206](#206)) ([fe44774](fe44774)) * `LdtkProject::project` and `LdtkLevel::level` fields have both been renamed to `data` ([#206](#206)) ([fe44774](fe44774)) * `LevelMap` and `TilesetMap` type aliases have been removed ([#206](#206)) ([fe44774](fe44774)) * All fields of `LdtkProject` and `LdtkLevel` are now privatized, and have immutable getter methods ([#206](#206)) ([fe44774](fe44774)) * asset `Loader` types are now private ([#206](#206)) ([fe44774](fe44774)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Trevor Lovell <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I've added a few different IntGrids to track different types of tile metadata and noticed that the result is essentially a new entity hierarchy, tilemap, etc. for each grid under the level entity.
I appreciate that we can make the IntGrid tiles invisible, but in my particular case, I really have no use for these entities at all, and I'm a little concerned about their effect on performance. I'm not using them for AutoLayers, I don't need to see them in the game world (because I can see them very easily in LDtk itself), and there is a lot of redundant data being spawned - hundreds of entities each with dozens of components, many of which are going to trigger some tilemap systems and other systems to run despite being invisible to the eye. It's compounded when I'm loading up to 4 level neighbors.
Would it be possible to add something to the
LdtkSettings
, or wherever makes the most sense, that allows either ignoring all IntGrid layers (except AutoLayers, I guess) or selectively whitelisting/blacklisting the layers we're interested/not interested in?I'd like to also point out that when using IntGrid as metadata - which is kind of the point, AutoLayers weren't implemented until much later - it is usually not faster to query the ECS than it is to just query the level data in the LDtk project, because the ECS is not indexed by any useful key. Whereas, in the LDtk project itself, if we know the grid coordinates of the tile we want to query, and also know the grid size, then it is simple and cheap math to translate that into an index and no problem at all to do this a few hundred times per frame.
(P.S. I totally get that it will be useful for some users to get a whole entity tree for those layers, maybe they want to run additional systems or spawn new entities based on it, etc., and those are better optimized in ECS style. That's why I'm asking for it to be a choice.)
(P.P.S. If you think it's a useful request, but don't have time/are busy with other priorities, I'm willing to take a crack at a PR if you could give me a few pointers re: where the magic happens.)
The text was updated successfully, but these errors were encountered: