Skip to content
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

A Rewrite #170

Closed
StarArawn opened this issue Apr 11, 2022 · 22 comments · Fixed by #180
Closed

A Rewrite #170

StarArawn opened this issue Apr 11, 2022 · 22 comments · Fixed by #180
Labels
code quality Make the code faster or prettier enhancement New feature or request help wanted Extra attention is needed usability Improve the ergonomics of the end user API

Comments

@StarArawn
Copy link
Owner

Hello author here!

I wrote an article about the state of tilemaps in bevy. Mostly around my experience with creating bevy_ecs_tilemap and its pitfalls. You can find it here:
https://startoaster.netlify.app/blog/tilemaps/

My main takeaway is that bevy_ecs_tilemap needs a re-write in order to facilitate a better API design. Feel free to leave any questions you have here!

Be on the lookout for a branch called rewrite. I'll comment here as well when I push it up. This branch will track the changes I make and will be a from the ground up re-write of the entire library. Feature wise I'll be focusing on the basics and moving on to add more features as I go.

What does this mean for the code base currently? I will no longer be accepting any PR's that add new features. This is to reduce the amount of work I'll end up needing to do during this re-write period. I will still accept bug fixes.

If you would like to see any new features, or suggest changes to the new API design, feel free to leave them here!

Thanks!

@StarArawn StarArawn added enhancement New feature or request help wanted Extra attention is needed code quality Make the code faster or prettier usability Improve the ergonomics of the end user API labels Apr 11, 2022
@rparrett
Copy link
Collaborator

Are you planning on updating the plugin in its current state for bevy 0.7?

@StarArawn
Copy link
Owner Author

Are you planning on updating the plugin in its current state for bevy 0.7?

I'm not sure when bevy 0.7 will come out. Regardless I think an update will happen.

@rparrett
Copy link
Collaborator

rparrett commented Apr 12, 2022

We are being told perhaps tomorrow Friday.

@ghost
Copy link

ghost commented Apr 14, 2022

This sounds great, thanks for all the hard work you're putting into this. The most important feature to me is performant large multi-layered isometric tile maps. The only thing that comes to mind from an API perspective is keeping anything chunk related out of the public API as much as possible, it feels like an implementation leak, but I think you;re already trying to do this.

I wonder if geometry shaders still suck 7 years later?

I haven't had to do any low level graphics work in a long long time, but if there is anything more general that would be good for someone else to put PRs in for, I'd be willing to try and help out.

@StarArawn
Copy link
Owner Author

StarArawn commented Apr 14, 2022

The most important feature to me is performant large multi-layered isometric tile maps.

I have a newer isometric tile map renderer I'm using for a game I'm making. It can handle upwards of 1m tiles at 30fps. There are some downsides I've discovered:

  • As bevy entity counts increase rendering performance decreases. At 40m tiles for example I was rendering at 5fps most of which was being consumed on the CPU side by bevy's internal logic. I filed a bug report here:
    High entity counts cause decreased rendering performance even when rendering nothing. bevyengine/bevy#3953 I think there's been a little bit of progress on it.
  • Mapping screen coordinates to 3D tile coordinates is a nightmare. I have a somewhat working solution.
  • Biggest issue is that bevy's sprite renderer does not have a depth buffer. Which means that there is no way to quickly sort tiles. Because of this you have to either create your own sprite renderer(not very user friendly), or slow down the tile renderer by splitting each row of tiles into its own mesh/chunks so that they can have the correct sorting applied. Sorting is typically the Y value of the tile after projection if I remember right.

A screenshot:
image

I wonder if geometry shaders still suck 7 years later?

Yep, generally most AAA games shy away from these days. A few other reasons not to use them:

  • No WebGPU/WebGL support.
  • Geometry shaders are being replaced by mesh shaders (no wide support for them either yet really though).

I haven't had to do any low level graphics work in a long long time, but if there is anything more general that would be good for someone else to put PRs in for, I'd be willing to try and help out.

Right now I need some ideas on how I can make tile removal more pleasant. I had planned on doing it when you despawn an entity, however I can't really get any data from a despawn event which means we have a few different API choices we could make here:

  • Add a RemoveTile component that allows me to know when a Tile is being removed and to remove it from the renderer and later from the world.
  • Keep a cache of data for each tile that I can use when a tile is despawned. More memory would be used but users could just despawn tile entities like normal.
  • Some other method I haven't thought of yet?

@inodentry
Copy link
Contributor

inodentry commented Apr 16, 2022

@StarArawn I'd like to give a warning about rewrites: they can easily turn into a daunting and demoralizing task and cause burn-out. It can literally kill your project.

You have already spent many months developing the current version of the crate. Are you really convinced that you are going to have sustained long term mental energy (and time and dedication) to drive a rewrite, even to feature parity with the current version? It's probably going to be a lot of work. If you really are ripping everything out and starting from scratch, because you want to change the project architecture that much, it's not likely much of the existing code would be reusable. Recreating something big (that is already quite impressive), that you have already done before, can feel pretty tedious and even demoralizing, as there is little novelty (you gotta reimplement all the stuff that was already working, before you can get to making cool new stuff).

I am talking from experience. Both from my own projects and from observing others. Complete rewrites rarely go well. They very often burn out and kinda die or lose momentum.

It's easy to imagine in your head how your project could probably be so much better, if only you could somehow not have to deal with all the technical debt and a bunch of legacy poor architectural decisions getting in the way ... but does that really have to call for a complete rewrite?

Especially with Rust, where "compiler-driven development" is a thing, and you can relatively confidently do major re-architecturing and then largely just fix errors until it works again ...

Taking something working, that you can hack on, and breaking and fixing it over and over again as needed, etc ... is often so much more productive than starting from scratch. You get the mental satisfaction of improving the thing ... rather than throwing out all your previous hard work.

I strongly encourage you to reconsider.

Look at Bevy: it managed to literally replace its heart: the whole ECS, and is now in the process of replacing its core scheduling ... and that was largely possible because there was all the other stuff – there was something working to come back to, after you fix the major breakage you caused. If Bevy had decided to abandon its course and do a complete from-scratch rewrite in order to improve its core foundations ... it probably would have died as a project.

@StarArawn
Copy link
Owner Author

I am talking from experience. Both from my own projects and from observing others. Complete rewrites rarely go well. They very often burn out and kinda die or lose momentum.

I should be a bit more careful with my words I guess. Yes I'm doing a complete re-write but a large portion of the renderer is being copy/pasted. I'm also reusing as much logic as I can like this:
https://github.com/StarArawn/bevy_ecs_tilemap/blob/main/src/neighbors.rs#L68
Which will be largely kept just moved into a new location and tweaked to work with the new API.

The one spot where I can't really facilitate reuse is the 3d iso stuff and I've mentioned some of the hardships I had there. I'm close to having square tile maps at a point where I can release it into the wild and get feedback on the new API. At which point adding the other 2D tile types should be straight forward as they don't really change logic that much outside of the shaders(which are already written).

Eventually I would like to re-write the renderer for 2d tile maps as well to swap over to vertex pulling, but that's not on the top of my agenda at the moment. As for 3D tilemaps, I.E. stacked isometric, stacked hexagon, I've largely already written the code necessary for that and then some(Including some basic 3d isometric tile picking). Example:

https://i.imgur.com/wdi78vd.mp4

Especially with Rust, where "compiler-driven development" is a thing, and you can relatively confidently do major re-architecturing and then largely just fix errors until it works again ...

My approach so far has been to keep existing code intact. It gives me a clear baseline and a place to work off of.

@inodentry
Copy link
Contributor

OK. Sorry for being over-dramatic.

Sounds like you are confident in the process and it's going well. :)

Wish you all the best! Looking forward to the improvements. :)

@StarArawn
Copy link
Owner Author

OK. Sorry for being over-dramatic.

I don't think you were overly dramatic. It's important to highlight issues with any development. I'm certainly not the best developer and I'm always looking for opportunities to learn new things!

@StarArawn
Copy link
Owner Author

StarArawn commented Apr 19, 2022

I have a very early WIP branch which can be found here:
https://github.com/StarArawn/bevy_ecs_tilemap/tree/rewrite

Most square tile functionality is working, but there are a few things to change cleanup with the internals. I also need to write more API around accessing tiles.

@sggt
Copy link

sggt commented Apr 20, 2022

Are you planning on updating the plugin in its current state for bevy 0.7?

I'm not sure when bevy 0.7 will come out. Regardless I think an update will happen.

Is the new version 0.6 for bevy_ecs_tilemap ready for release or there are any problems with it? Is there a way to link to the trunk version of a dependency in the cargo.toml? Sorry, I'm just a beginner to the rust ecosystem and sorry for the out of topic comment.

@StarArawn
Copy link
Owner Author

StarArawn commented Apr 20, 2022

Is the new version 0.6 for bevy_ecs_tilemap ready for release or there are any problems with it? Is there a way to link to the trunk version of a dependency in the cargo.toml? Sorry, I'm just a beginner to the rust ecosystem and sorry for the out of topic comment.

Main should be compatible with bevy 0.7. I'll be rolling a new release soon. 👍

@LeonardMH
Copy link

@sggt, hopefully we'll get 0.6 as an official release soon, but if you need this crate in the meantime you can specify that cargo pulls directly from the main branch of the repo. See this section of the cargo docs where this is explained.

My project is not complex but I confirmed that after updating my Cargo.toml appropriately my project now builds against bevy 0.7. Relevant Cargo.toml update below:

# this should change to `bevy_ecs_tilemap = "0.6.0"` whenever that is released
bevy_ecs_tilemap = { git = "https://github.com/StarArawn/bevy_ecs_tilemap.git", branch = "main" }

@StarArawn
Copy link
Owner Author

@sggt, hopefully we'll get 0.6 as an official release soon, but if you need this crate in the meantime you can specify that cargo pulls directly from the main branch of the repo. See this section of the cargo docs where this is explained.

My project is not complex but I confirmed that after updating my Cargo.toml appropriately my project now builds against bevy 0.7. Relevant Cargo.toml update below:

# this should change to `bevy_ecs_tilemap = "0.6.0"` whenever that is released
bevy_ecs_tilemap = { git = "https://github.com/StarArawn/bevy_ecs_tilemap.git", branch = "main" }

The 0.6 release is live. 👍

@modulozero
Copy link

The 0.6 release is live. 👍

FYI it's not on Github releases, which was a tiny bit confusing for a moment.

@StarArawn
Copy link
Owner Author

The 0.6 release is live. 👍

FYI it's not on Github releases, which was a tiny bit confusing for a moment.

Ah yes I'll add one shortly.

@freiksenet
Copy link

freiksenet commented Jun 4, 2022

Hi! I really like the new API, I'd like to readd support for animated tiles and color materials, do you reckon it's okay if I try to contribute those parts in? In that case, did you plan to have similar API for them (component to enable animation and passing color to tiles for color material).

@StarArawn
Copy link
Owner Author

Hi! I really like the new API, I'd like to readd support for animated tiles and color materials, do you reckon it's okay if I try to contribute those parts in? In that case, did you plan to have similar API for them (component to enable animation and passing color to tiles for color material).

Yeah that would be a great help! I haven't had as much time as I'd like lately, but I'm planning on finishing up the rewrite soon. :)

@JNEBL
Copy link

JNEBL commented Jun 8, 2022

Hey did bevy ever fix the performance problems with having large number of entities

@InnocentusLime
Copy link

InnocentusLime commented Jul 20, 2022

Hello! Will the rewrite have a more advanced/flexible animation feature?

@StarArawn
Copy link
Owner Author

Hey did bevy ever fix the performance problems with having large number of entities

Sadly not yet as far as I am aware.

Hello! Will the rewrite have a more advanced/flexible animation feature?

Not initially but it's something I would like to support! It just requires sending a bit more information to the GPU.

@rparrett
Copy link
Collaborator

How's the rewrite progressing? Are you planning on updating the crate for Bevy 0.8 with the main branch code?

I updated the main branch code for bevy main here: https://github.com/rparrett/bevy_ecs_tilemap/tree/bevy-0.8 and would be happy to PR if that's the plan.

Bevy 0.8 is likely to land this week.

@StarArawn StarArawn mentioned this issue Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Make the code faster or prettier enhancement New feature or request help wanted Extra attention is needed usability Improve the ergonomics of the end user API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants