apotapov / gdx-artemis

Fork of artemis entity-system (http://gamadu.com/artemis/) refactored to integrate with Libgdx (http://libgdx.badlogicgames.com/)
Other
53 stars 5 forks source link

createEntityInstance in EntityManager increments entity id's twice #40

Closed vkvam closed 10 years ago

vkvam commented 10 years ago

When creating new entities, identifierPool.checkOut() is called twice, once from super.obtain(), and once from newObject(). Since newObject() shouldn't set the entity id, I created an Entity constructor without id parameter, instead of setting a bogus id in newObject().

Sorry about the multiple commits.

apotapov commented 10 years ago

Thanks for the pull request. There are a lot of changes coming in.

I see that it's mainly the id and changing to libgdx Bits from BitSet. I want to take a look through more thoroughly. It would also be helpful if you added some unit tests for this issue.

apotapov commented 10 years ago

The Entity id fix looks good. I guess I never paid attention to them. But I could see how it can cause trouble since we are incrementing id's twice but decrementing them only once. Good catch.

As for moving from BitSet to Bits, it seems like a pretty safe change since it's an imports only change. And I'm guessing both this change and removal of UUID puts this project closer to GWT compatibility. So even though it changes some of the public API, I believe it's a step in the right direction.

Thanks for the good work.

vkvam commented 10 years ago

Glad to contribute a bit, although the Bitset->Bit originally came from "deepinthewoods", I just merged it into my fork. My only thought about it at the moment, is that libgdx classes should be reused to the greatest extent possible. If I find time to get my head around where and how BitSets are used, I'll look into creating some unit tests and add a new pull request. Thanks for an excellent gdx adaptation of artemis.

apotapov commented 10 years ago

No worries. I meant it more for ids than the bitset. I assume bitset works as advertised inside libgdx. Not a big deal either way. I was hoping to shore up a test suite for the library though. And best time to do it is when adding new features and fixing bugs. But all good, I'm glad someone is finding this library useful.