Fixed order_by when using a shared component by multiple prefabs

[klechenov]

I have a component

struct VisualComponent {
	int zOrder;
	Texture2D texture;
};

which I use as a common component for my game entities. Game entities are implemented through Prefab.

ecs.prefab<AsteroidPrefabTag>("PrefabAsteroid")
        .add<AsteroidTag>()
        .emplace<VisualComponent >(2, LoadTexture("res/asteroid.png"))
        .emplace<Size>(ASTEROID_WIDTH, ASTEROID_HEIGHT);

ecs.prefab<RocketPrefabTag>("PrefabRocket")
	.add<RocketTag>()
	.emplace<Visual>(0, LoadTexture("res/rocket.png"))
	.emplace<Size>(ROCKET_WIDTH, ROCKET_HEIGHT);

Since VisualComponent is not modified for different instances, I add it to the prefab. However, its data differs between different prefabs.

The issue arose when I tried to use this component in order_by.

ecs.system<const VisualComponent, const PositionComponent, const SizeComponent, const RotationComponent>("Renderer::DrawEntities")
			.order_by<const VisualComponent>(
					[](flecs::entity_t e1, const VisualComponent* p1,
					   flecs::entity_t e2, const VisualComponent* p2) {
							return (p1->zOrder > p2->zOrder) - (p1->zOrder < p2->zOrder);
					})
			.each([](...});

order_by works incorrectly due to this check in the order_by logic:

  if (column == -1) {
      /* Component is shared, no sorting is needed */
      dirty = false;
  }

Because of it, when creating a new instance, no resorting occurs.

I understand that this check was added because if prefab instances for sorting use a shared component, this component will be the same for all instances, so there is no point in resorting. But there is also no point in such sorting in principle. So, using order_by with a shared component and when you have only one prefab makes no sense.

However, if you have a component that is used in several different prefabs and has different data in these prefabs, then this check breaks the logic of how order_by works, as in my example above.

So, my suggestion is to remove this check so that order_by can be used for my case

[SanderMertens]

Can you provide a runnable code example that reproduces your scenario? This change breaks existing behavior- but maybe there's a way to make both work.

[klechenov]

@SanderMertens yeah, sure. here it is - https://gist.github.com/klechenov/a3708b1385704871d4bbf04634c6d4fc

the result without order_by will be:

Rocket
Rocket
Rocket
Rocket
Asteroid
Asteroid
Asteroid
Asteroid




Rocket
Rocket
Rocket
Rocket
Rocket
Rocket
Rocket
Rocket
Asteroid
Asteroid
Asteroid
Asteroid
Asteroid
Asteroid
Asteroid
Asteroid

the result with order_by will be:

Asteroid
Asteroid
Asteroid
Asteroid
Rocket
Rocket
Rocket
Rocket




Asteroid
Asteroid
Asteroid
Asteroid
Rocket
Rocket
Rocket
Rocket