junkdog / artemis-odb

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

SubscriptionListener doesn't inform about first entities #248

Closed Namek closed 9 years ago

Namek commented 9 years ago

When I register EntitySubscription.SubscriptionListener (through ugly reflected inspection) I get informed only about entities added/removed during the game.

I've got GameSessionManager extends Manager which has overriden initialize() method. That method adds some entities to the world. The problem is that I don't get informed about those entities added in through that method. @junkdog Is that correct behavior? If it is, then probably any workaround to get information about ALL added/removed entities?

Side note: I'm working on my custom entity tracker (probably something similiar to dominatrix-odb)

junkdog commented 9 years ago

(through ugly reflected inspection)

Why do you have to use reflection? It's possible I've left something out of the code, but:

  1. Make sure your manager implements SubscriptionListener
  2. get the EntitySubscription from AspectSubscriptionManager
  3. register your manager: EntitySubscription#addSubscriptionListener

The above will/should only inform you about which entities have been added/removed from the EntitySubscription. If you want to be informed about all entities, override the EntityObserver stubs in Manager (added, changed, deleted).

Be aware that the EntitySubscription code hasn't been finalized yet, so you're on the extreme end of bleeding edge atm. I figured I'd try to polish it once everything is working; currently resolving a profile weaver bug.

Namek commented 9 years ago

AspectSubscriptionManager has private things, the only way to get EntitySubscription is by having Aspect.Builder which is private in EntitySystem. So I get Aspect.Builder using reflection.

If you want to be informed about all entities, override the EntityObserver stubs in Manager (added, changed, deleted).

I tried to override any/both of #inserted methods in EntitySystem and I think there's a bug because I don't get notified about entities at all. Overriding added in Manager works, thanks.

I know it's a bleeding edge. I rather wait for your API to be finalized but above (first entities creation is not broadcasted) seems like a bug to me so I decided to ask.

junkdog commented 9 years ago

I'll see to it first thing tomorrow; been lazy with test cases... heh.

junkdog commented 9 years ago

Could you try it now - it's on the extract-base-system-from-es branch? I I think I have it working, unless I've missed an edge case.

edit: merged to master

junkdog commented 9 years ago

AspectSubscriptionManager has private things, the only way to get EntitySubscription is by having Aspect.Builder which is private in EntitySystem. So I get Aspect.Builder using reflection.

I've locally added a getAspectBuilder method to EntitySubscription. While maybe not what you're looking for, you can pass a new Aspect.Builder to AspectSubscriptionManager in order to retrieve the EntitySubscription instance.

junkdog commented 9 years ago

@Namek Do you have an RSS feed to your blog?

DaanVanYperen commented 9 years ago

http://www.namekdev.net/feed/?

junkdog commented 9 years ago

That was easy, don't know why my rss reader didn't pick it up.

Namek commented 9 years ago

Could you try it now - it's on the extract-base-system-from-es branch? I I think I have it working, unless I've missed an edge case.

Well, ok. First - I like results of #247! But I wonder why VoidEntitySystem does extend BaseSystem and not EnitySystem (hence the name). This way I can't simply implement inserted and removed when I have no aspect and still want to have some kind of update() method (currently: processSystem()). Instead, I can...

While maybe not what you're looking for, you can pass a new Aspect.Builder to AspectSubscriptionManager in order to retrieve the EntitySubscription instance.

and I did that this way:

AspectSubscriptionManager am = world.getManager(AspectSubscriptionManager.class);
am.get(Aspect.all()).addSubscriptionListener(this);
public class EntityTracker extends VoidEntitySystem implements EntitySubscription.SubscriptionListener { // ...

and works :) so I'm closing this issue. Thanks!

Namek commented 9 years ago

@junkdog Side question: will Managers even exist in 1.0? Those EntitySubscriptions look quite like a replacement (but you would need to do something with processSystem(), I think that's the only major difference between systems and managers).

DaanVanYperen commented 9 years ago

ut I wonder why VoidEntitySystem does extend BaseSystem and not EnitySystem (hence the name).

Arni described it in javadoc as a class just for entityless and aspectless game logic. Your comment makes sense though, there's probably a lot of legacy around this class.

Why not move it back into EntitySystem hierarchy for legacy sake and train people to prefer BaseSystem instead (I'm loving BaseSystem btw).

Namek commented 9 years ago

@DaanVanYperen @junkdog another side question: how to call managers and basesystems by one common name? Previously I could tell that they're entity observers but after those cool BaseSystem appeared...

junkdog commented 9 years ago

Why not move it back into EntitySystem hierarchy for legacy sake and train people to prefer BaseSystem instead (I'm loving BaseSystem btw).

Yup, it makes more sense; re-opening the issue until it's fixed. Could just deprecate VoidEntitySystem.

@Namek atm, it isn't possible; it might come in the future, but I can't decide if it's adding any value or just breaking code unnecessarily - I go back and forth. The new BaseSystem is so bare-bones that it can serve as a Manager too, except for the missing EntityObserver interface. World#setSystem should probably intercept systems implementing EntityObserver either way.

Namek commented 9 years ago

Just reporting - subscriptions still don't work as expected. I use v0.9.1-SNAPSHOT right now.

Even if I get notified for Aspect.all(), I don't get notified about other aspects, like Aspect.all(Renderable.class).

So even in EntitySystem I have to manually register subscription listener:

protected void initialize() {
  AspectSubscriptionManager am = world.getManager(AspectSubscriptionManager.class);
  am.get(Aspect.all()).addSubscriptionListener(this);
}
Namek commented 9 years ago

Another note:

world.createEntity().edit().create(Renderable.class);

When I get notified about this entity, it doesn't have Renderable component (e.getComponent(Renderable.class) returns null). So it would be cool to be informed about new entity in next frame.

junkdog commented 9 years ago

Thanks for reporting. Do you know if the bug occurs in 0.9.0 too (alt I can check tomorrow)?

Hmm, might try to do an early release 0.10.0 beginning of next week (postponing world serialization until 0.11.0). There are a couple of other bugs I want resolved too + working subscription listeners would be neat.

On Fri, Jun 19, 2015 at 5:35 PM, Namek notifications@github.com wrote:

Another note:

world.createEntity().edit().create(Renderable.class);

When I get notified about this entity, it doesn't have Renderable component (e.getComponent(Renderable.class) returns null). So it would be cool to be informed about new entity in next frame.

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/248#issuecomment-113550188 .

Namek commented 9 years ago

Ahh, sorry. Did you check it already?

Namek commented 9 years ago

I can confirm that subscribing for specific Aspect doesn't get me notified about insertion in 0.9.0.

I put systems in this order:

  1. System1 extends BaseSystem
  2. System2 extends BaseSystem implements SubscriptionListener.

System1:

    @Override
    protected void processSystem() {
        if (!isInitialized) {
            world.createEntity().edit().create(MyComponent.class);
            isInitialized = true;
        }
    }

Now System2:

    @Override
    protected void initialize() {
        AspectSubscriptionManager am = world.getManager(AspectSubscriptionManager.class);
        am.get(Aspect.all(MyComponent.class)).addSubscriptionListener(this);
    }

    @Override
    public void inserted(ImmutableBag<Entity> entities) {
        MyComponent cmp = mapper.get(entities.get(0));
        // breakpoint never gets me here
    }
junkdog commented 9 years ago

I can confirm that subscribing for specific Aspect doesn't get me notified about insertion in 0.9.0.

But the above works in 0.9.1-SNAPSHOT, right? If not, how does this differ?

Namek commented 9 years ago

AFAIR it doesn't

junkdog commented 9 years ago

MyComponent cmp = mapper.get(entities.get(0)); // breakpoint never gets me here

Did you add @Wire to System2 btw?

junkdog commented 9 years ago

Possibly fixed by #268

Namek commented 9 years ago

Not sure about @Wire then. I'll be testing that in following days. Gonna report if that's still broken.

Namek commented 9 years ago

I can't reproduce this bug anymore. I use artemis quickstart and Gradle, even if I put artemisVersion=0.9.0 it still downloads 0.9.1-SNAPSHOT dated at April 8th.

The #268 was just another bug, I think this one here was fixed earlier in 0.9.1-SNAPSHOT.

Anyway, the initial problem seems to be fixed. Thanks :)

junkdog commented 9 years ago

Nice, well - I better release 0.10.0 then.