feature: migrate to required components

[patrickariel]

Closes #597

Objective

• Required components replaces bundles, but the crate still contains many user-facing bundles.

Solution

• Deprecate the following bundles:

  • TileBundle
  • MaterialTilemapBundle
  • TilemapBundle
  • StandardTilemapBundle

• Migrate all examples to required components.

• Deprecate MaterialTilemapHandle in favor of TilemapMaterial. This is a pure name change done for two reasons:

  • To conform to the idiomatic pattern used upstream:

    commands.spawn((
        Mesh3d(shape),
        MeshMaterial3d(material),
    ));

  • The material component is now something that is explicitly inserted by the user. So, we need to make sure that it's not cumbersome to remember and type.

Testing

I've ran all the tests on Linux (Wayland) and they all look fine to me. But I'm not 100% familiar with all the quirks and edge-cases of this crate. So if I've overlooked any potential regressions, do let me know 🙂

Migration Guide

The bundles are now all deprecated. With the new version, you should instead compose together Tile, Tilemap and TilemapMaterial to achieve your desired behavior.

Before:

commands.spawn(TileBundle::default());

commands.spawn(MaterialTilemapBundle {
    material,
    ..default(),
});

commands.spawn(TilemapBundle::default());

commands.spawn(StandardTilemapBundle::default());

After:

commands.spawn(Tile);

commands.spawn((Tilemap, TilemapMaterial(material)));

commands.spawn((Tilemap, TilemapMaterial::standard()));

commands.spawn(Tilemap);

[ConnerPetzold]

I think to maintain feature parity, you'd need to create two versions of the Tilemap component based on the render feature.

[patrickariel]

I believe the feature split shouldn't be necessary anymore. StandardTilemapBundle (not(feature = "render")) was just MaterialTilemapBundle (feature = "render") without TilemapMaterial.

[ConnerPetzold]

I wonder if Tilemap should require TilemapMaterial<StandardTilemapMaterial>? I'm not sure how generics work with required components but it seems like the vast majority of users won't need a custom material.

[patrickariel]

I think this would sort of complicate the API surface, because now we would need a TilemapWithoutMaterial for users who wants custom material or no material (e.g. not(feature = "render")).

Having the material component be opt-in feels a lot more natural and idiomatic, in line with how it is handled upstream:

commands.spawn((
    Mesh3d(shape),
    MeshMaterial3d(material),
));

Here, Mesh3d does not automatically require MeshMaterial3d<M>. It must be inserted manually by the user.

[ConnerPetzold]

Hmm yeah I was only thinking about the use case of a custom material and not about the non-rendering case. I suppose that case will be more common for headless/backend.

[ConnerPetzold]

Although why not just make Tilemap conditionally require TilemapMaterial<StandardTilemapMaterial> based on the render flag to mirror what happens now? That way you don't need two separate components. And I think it still covers the use case of a custom material because I believe you'd still be able to override with a custom TilemapMaterial, though I'm not 100% sure about that.

[teohhanhui]

I believe you'd still be able to override with a custom TilemapMaterial

That's correct.

[patrickariel]

The idea sounds very attractive, but I suspect that it isn't possible due to the generics here.

The problem here is that there's no way to know if the MaterialTilemapHandle<M> component is actually missing, because there is an infinite amount of possible permutations (due to the generics).

In other words, something like Without<TilemapMaterial<M>>, where M is every material type possible, is simply not possible under the current type system.

For example, let's take queue_material_tilemap_meshes<M> here:

bevy_ecs_tilemap/src/render/material.rs

Lines 414 to 417 in 843f7be

	(standard_tilemap_meshes, materials): (
	Query<(Entity, &ChunkId, &Transform, &TilemapId)>,
	Query<&MaterialTilemapHandle<M>>,
	),

The solution seems simple at first: just change Query<&MaterialTilemapHandle<M>> to Query<Option<&MaterialTilemapHandle<M>>> and test for its presence in the code. If it's None, use StandardMaterialTilemap. But this gets complicated when the user have multiple materials, like:

• queue_material_tilemap_meshes::<StandardMaterialTilemap>
• queue_material_tilemap_meshes::<Foo>

The consequence is that logic that used to run only when a specific material type is present, will now always run:

bevy_ecs_tilemap/src/render/material.rs

Lines 459 to 464 in 843f7be

	let Ok(material_handle) = materials.get(tilemap_id.0) else {
	continue;
	};
	let Some(material) = render_materials.get(&material_handle.id()) else {
	continue;
	};

Imagine how the system would behave against entities like:

• (Tilemap, MaterialTilemapHandle<StandardMaterialTilemap>)
• (Tilemap, MaterialTilemapHandle<Foo>)
• (Tilemap, MaterialTilemapHandle<Bar>)
• (Tilemap,)

This will introduce a plethora of race conditions where the render system for each material type will try to queue up StandardMaterialTilemap, because the specific material type they were instantiated for is missing.

I appreciate the idea, but if you have any potential solutions here do let me know.

[ConnerPetzold]

Ah yeah I see, that's a tricky problem.. What about something like this:

#[cfg(feature = "render")]
#[derive(Component, Debug, Default, Clone)]
#[require(
    TilemapGridSize,
    TilemapType,
    TilemapSize,
    TilemapSpacing,
    TileStorage,
    TilemapTexture,
    TilemapTileSize,
    Transform,
    TilemapRenderSettings,
    TilemapAnchor,
    Visibility,
    FrustumCulling,
    SyncToRenderWorld
)]
pub struct TilemapMaterial<M: MaterialTilemap>(pub Handle<M>);

#[cfg(feature = "render")]
pub type Tilemap = TilemapMaterial<StandardTilemapMaterial>;

#[cfg(not(feature = "render"))]
#[derive(Component, Debug, Default, Clone)]
#[require(
    TilemapGridSize,
    TilemapType,
    TilemapSize,
    TilemapSpacing,
    TileStorage,
    TilemapTileSize,
    Transform,
    Visibility,
)]
pub struct Tilemap;

[teohhanhui]

@ConnerPetzold That just doesn't make sense and is super confusing for users. Why not just stick to the same pattern as Bevy? As a user I'd like better DX too, but I think this should be done at the Bevy level.

[ConnerPetzold]

I don't understand why it's confusing for users:

(TIlemap) // use the standard material if render=true, otherwise no materials/rendering
(TilemapMaterial<Custom>) // use a custom material, and will only be available if render=true

It's mirroring what ecs_tilemap has now, and essentially just collapses what you have with Tilemap and TilemapMaterial. I don't think the MeshMaterial3d pattern applies here because writing a custom tilemap material will be very rare. I think the more appropriate Bevy pattern to follow here would be Sprite, since like tilemaps you almost never need a custom material (and I guess in the case of Sprite, you can't even use a custom material). For the rare cases you do, you have the mesh/material escape hatch.

[teohhanhui]

I guess that would make sense if it's called TilemapWithMaterial.

[suprohub]

any updates ?