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

Engines should provide a check for if an entity is present, at least in tests #52

Open
Derongan opened this issue Nov 18, 2021 · 2 comments

Comments

@Derongan
Copy link
Member

I noticed in your code you do some hacky stuff by using Geary.getNextId to figure out if an entity has been removed. This is downright barbaric. It only makes sense if we assume that the engine always uses the lowest free slot for the next entity. You are relying on a hidden implementation detail in tests.

Additionally the use of Engine vs the constructed engine is really confusing and smelly.

@0ffz
Copy link
Member

0ffz commented Nov 20, 2021

Sorry can you elaborate on what you were trying to do originally?

The constructed engine stuff is just using spigots service system unless I'm missing the point ur making there too.

@Derongan
Copy link
Member Author

This is probably 2 bugs.

  1. Geary Engine should provide a method to check if a GearyEntityId is tracked by the engine. There are a number of tests that test for Entity removal that end up being very hacky because of lack of support for this feature. You instead grab the next available entity id and check to see if it is the slot that was associated with the deleted entity.

  2. The use of Engine rather than directly using implementation under test in geary tests is problematic. If we are testing the geary engine, we should not be creating it then relying on the service getter to fetch the engine during the test. We have a direct reference to the engine already. See QueryManagerTest for an example.

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

No branches or pull requests

2 participants