haxetink / tink_state

Handle those pesky states.
The Unlicense
29 stars 13 forks source link

Track source in hasChanged #50

Closed kevinresol closed 4 years ago

kevinresol commented 4 years ago

I am not sure about this actually.

I spent 2 full days trying to track down a bug in my ecs where my entity list is not refreshing upon a dependency change, and I came along this line of "uncertain" code. I am desperate for any solution so I tried to change the line and apparently it fixed my problem. (Because I am also unable to reduce the bug from my pretty complex dependency tree)

Here is my code (suggested by @back2dos in gitter) for reference:

public function query(q:Query):Observable<Array<Entity>> {
    final qs = q.toString();
    final entityQueries = {
        final cache = new Map<Int, Pair<Entity, Observable<Bool>>>();
        Observable.auto(() -> {
            for (id => entity in map)
                if (!cache.exists(id))
                    cache.set(id, new Pair(entity, Observable.auto(() -> entity.fulfills(q))));

            final deleted = [for (id in cache.keys()) if (!map.exists(id)) id];

            for (id in deleted)
                cache.remove(id);

            cache;
        },
            (_, _) -> false // we're always returning the same map, so the comparator must always yield false
        );
    }

    return Observable.auto(() -> [for (p in entityQueries.value) if (p.b) p.a]);
}

The issue was that at some very special scenario the computation of entity.fulfills(q) gets re-evaluated when the dependency changed but the comparator doesn't consider that as a change (confirmed with a custom comparator) because the old/new value are the same. I can't understand why that happens because I can't seem to capture where the flip of the boolean happened.

kevinresol commented 4 years ago

My bug still happens with this "fix"

back2dos commented 4 years ago

Hmm. I've experienced the same issue, although with more complex code.

The solution looks very odd though. Is there a public repo with that has the issue you're experiencing?

kevinresol commented 4 years ago

please see #51

back2dos commented 4 years ago

Awesome, thanks ;)