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

Fixed order_by when using a shared component by multiple prefabs #1086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

klechenov
Copy link

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
Copy link
Owner

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
Copy link
Author

@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

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.

2 participants