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

Rewrite #180

Merged
merged 49 commits into from
Aug 5, 2022
Merged

Rewrite #180

merged 49 commits into from
Aug 5, 2022

Conversation

StarArawn
Copy link
Owner

@StarArawn StarArawn commented Apr 20, 2022

Closes: #170
Closes: #205
Closes: #140
Closes: #121
Closes: #71

Comment on lines 38 to 39
layer_builder.for_each_tiles_mut(|tile_entity, tile_data| {
// True here refers to tile visibility.
*tile_data = Some(TileBundle::default());
// Tile entity might not exist at this point so you'll need to create it.
if tile_entity.is_none() {
*tile_entity = Some(commands.spawn().id());
for x in 0..640u32 {
for y in 0..640u32 {
let tile_pos = TilePos2d { x, y };
let tile_entity = commands
.spawn()
.insert(tile_pos)
.insert(TileTexture(0))
.insert(TilemapId(tilemap_entity))
.insert(LastUpdate::default())
.insert(TileVisible(true))
.id();
tile_storage.set(&tile_pos, Some(tile_entity));
}
commands
.entity(tile_entity.unwrap())
.insert(LastUpdate::default());
});
}

Choose a reason for hiding this comment

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

🙏🏻

To the completely ignorant of game dev person that I am, this is so much more intuitive. Can't wait for the post-rewrite version.

src/tiles/mod.rs Outdated
Comment on lines 63 to 69
pub struct TileBundle {
pub position: TilePos2d,
pub texture: TileTexture,
pub tilemap_id: TilemapId,
pub visible: TileVisible,
pub flip: TileFlip,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any play to support colors on tiles, like the old Tile component?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for implementing this!

@rparrett
Copy link
Collaborator

rparrett commented Aug 4, 2022

FWIW, I migrated a project to the rewrite branch and didn't have any issues. But I'm pretty much just loading a single static Tiled map.

@StarArawn
Copy link
Owner Author

FWIW, I migrated a project to the rewrite branch and didn't have any issues. But I'm pretty much just loading a single static Tiled map.

That's great news! Thanks for letting me know! 🙂

@StarArawn StarArawn marked this pull request as ready for review August 5, 2022 16:30
@StarArawn StarArawn merged commit e2adb45 into main Aug 5, 2022
@StarArawn StarArawn deleted the rewrite branch August 5, 2022 23:33
@neocturne
Copy link
Contributor

@StarArawn I think commit 926ceea would still be a good improvement for the rewrite. Do you want to cherry-pick it, or should I open a new PR?

@StarArawn
Copy link
Owner Author

So, according to one of Bevy's rendering devs that import is going to be removed. It was suggested to just keep what I have for right now.

@neocturne
Copy link
Contributor

I see! In that case the struct definition should be updated to https://github.com/bevyengine/bevy/blob/6752c9c59b3b81168ea6607767cbb519e5127976/crates/bevy_sprite/src/mesh2d/mesh2d_view_types.wgsl to match the Rust side of ViewUniform (this shader import was actually broken in the Bevy 0.8 release and fixed in a subsequent commit)

@StarArawn
Copy link
Owner Author

StarArawn commented Aug 6, 2022

I see! In that case the struct definition should be updated to https://github.com/bevyengine/bevy/blob/6752c9c59b3b81168ea6607767cbb519e5127976/crates/bevy_sprite/src/mesh2d/mesh2d_view_types.wgsl to match the Rust side of ViewUniform (this shader import was actually broken in the Bevy 0.8 release and fixed in a subsequent commit)

Ideally I suppose we should use bevy's import. Until we can I think copying it into our shader is probably best. Edit:
Here is a link to the new PR which fixes some of the issues: bevyengine/bevy#5535

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A Rewrite
6 participants