junkdog / artemis-odb

A continuation of the popular Artemis ECS framework
BSD 2-Clause "Simplified" License
779 stars 112 forks source link

DelayedEntityProcessingSystem expires all entites at once #32

Closed kanecko closed 11 years ago

kanecko commented 11 years ago

This is an old bug from the original Artemis. I've tested this on the trunk version of artemis-odb.

"Unfortunately, the newer DelayedEntityProcessingSystem still has the problem I described in my other post. If you add an entity A with an expiration of, say 1 second at T1, and then add another entity B that has the same expiration of 1 second at T2, where T2 < T1 + 1 second, entity B will expire before it should... it will expire at the same time as entity A."

Source: http://slick.ninjacave.com/forum/viewtopic.php?f=28&t=5478&hilit=DelayedEntityProcessingSystem#p30809 "my other post": http://slick.ninjacave.com/forum/viewtopic.php?f=28&t=5477&p=30798#p30798

junkdog commented 11 years ago

Thanks, it had gone under the radar. I've committed a fix+test that I think solves the problem.

kanecko commented 11 years ago

I'm still having issues with it. I'm firing bullets at a constant rate (200ms), and they are expiring in groups of four (after 1s). Also, the very last bullet fired doesn't expire at all.

junkdog commented 11 years ago

I'm firing bullets at a constant rate (200ms), and they are expiring in groups of four (after 1s).

Hmm... during the same processing round? If so, could you write a unit test that exhibits that behavior? I can't recreate it myself.

Also, the very last bullet fired doesn't expire at all.

Looking into it.

kanecko commented 11 years ago

Hmm... during the same processing round?

Yeah, they disappear at the same time.

If so, could you write a unit test that exhibits that behavior? I can't recreate it myself.

I'll get on it, but you will have to be patient. I'm a beginner at source control + working test cases :)

kanecko commented 11 years ago

Ok, here are the tests I've written: https://gist.github.com/kanecko/7277699

I've included my own system and component classes for the test. I've tried simulating the events happening in my application. I've tested for the number of processed entities per round, the order of which they get processed and I've used smaller deltas in one variation of the test. (In these tests the order should make sense because there will always be only one entity that is processed at the time/round)

junkdog commented 11 years ago

Ok, here are the tests I've written: https://gist.github.com/kanecko/7277699

Thanks! Copied the test into the repo; modified the delta by a small amount to account for rounding errors.

I'm cautiously optimistic that the system works as intended now. The DelayedEntityProcessingSystem is a mess, took me a while to track down the bug(s). Should probably rewrite it from scratch sometime in the distant future.

kanecko commented 11 years ago

It seems to be working fine now. Cool stuff!