Closed endison1986 closed 1 year ago
enricostara
commented
1 year ago Hi @endison1986, thanks for the test and I will add a new unit test to cover these cases as well. They are probably not related to number #117 which I have to address first. Along with the heavy core refactoring I got some regressions not readily spotted due to lack of specific unit tests. I'm about to review some basic mechanics to pass all new tests, it will take a little more time.
endison1986
commented
1 year ago Yes, I agree with you, it's not related to number #117 Dominion-ECS is too hard to me Currently，so I only can test it and feedback to you Hope it gets better
endison1986
commented
1 year ago Hi @enricostara I analysised the source code and maybe found the reason. dev.dominion.ecs.engine.CompositionRepository
public Map<IndexKey, Node> findWith(Class<?>... componentTypes) {
if (LoggingSystem.isLoggable(loggingContext.levelIndex(), System.Logger.Level.DEBUG)) {
LOGGER.log(
System.Logger.Level.DEBUG, LoggingSystem.format(loggingContext.subject()
, "Find entities with " + Arrays.toString(componentTypes))
);
}
switch (componentTypes.length) {
case 0:
return null;
case 1:
Node node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[0])));
return node == null ? null : node.copyOfLinkedNodes();
default:
Map<IndexKey, Node> currentCompositions = null;
for (int i = 0; i < componentTypes.length; i++) {
node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[i])));
if (node == null) {
continue;
}
currentCompositions = currentCompositions == null ?
node.copyOfLinkedNodes() :
intersect(currentCompositions, node.linkedNodes)
;
}
return currentCompositions;
}
}when I createEntity by component A, but try to find with A.class and B.class, this method will return nodeMap only contains A.class.
When I return null, it seems that the problem is solved.
public Map<IndexKey, Node> findWith(Class<?>... componentTypes) {
if (LoggingSystem.isLoggable(loggingContext.levelIndex(), System.Logger.Level.DEBUG)) {
LOGGER.log(
System.Logger.Level.DEBUG, LoggingSystem.format(loggingContext.subject()
, "Find entities with " + Arrays.toString(componentTypes))
);
}
switch (componentTypes.length) {
case 0:
return null;
case 1:
Node node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[0])));
return node == null ? null : node.copyOfLinkedNodes();
default:
Map<IndexKey, Node> currentCompositions = null;
for (int i = 0; i < componentTypes.length; i++) {
node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[i])));
if (node == null) {
return null; // here
}
currentCompositions = currentCompositions == null ?
node.copyOfLinkedNodes() :
intersect(currentCompositions, node.linkedNodes)
;
}
return currentCompositions;
}
}Then I tested this code
public static class A {
}
public static class B {
}
public static class C {
}
public static class D {
}
public static void main(String[] args) {
var w = Dominion.create();
var s = w.createScheduler();
s.schedule(() -> w.findEntitiesWith(A.class, C.class).stream().forEach(rs-> System.out.println(rs.entity())));
s.tickAtFixedRate(1);
w.createEntity(new A(), new B());
w.createEntity(new C(), new D());
}I get this error
dominion.SystemScheduler invoke
java.util.NoSuchElementException: java.util.NoSuchElementException
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
at java.base/java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:562)
at java.base/java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:591)
at java.base/java.util.concurrent.ForkJoinTask.joinForPoolInvoke(ForkJoinTask.java:1042)
at java.base/java.util.concurrent.ForkJoinPool.invoke(ForkJoinPool.java:2639)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.SystemScheduler.forkAndJoin(SystemScheduler.java:130)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.SystemScheduler$Single.call(SystemScheduler.java:279)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.SystemScheduler$Single.call(SystemScheduler.java:256)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.util.NoSuchElementException
at java.base/java.util.concurrent.ConcurrentHashMap$ValueIterator.next(ConcurrentHashMap.java:3480)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.ResultSet.iterator(ResultSet.java:56)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.ResultSet.stream(ResultSet.java:85)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.CompositionRepository.lambda$main$3(CompositionRepository.java:252)
at dev.dominion.ecs.engine/dev.dominion.ecs.engine.SystemScheduler$2.compute(SystemScheduler.java:133)
at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)I think the reason is that the length of nodeMap returned by findWith is zero.
public Map<IndexKey, Node> findWith(Class<?>... componentTypes) {
if (LoggingSystem.isLoggable(loggingContext.levelIndex(), System.Logger.Level.DEBUG)) {
LOGGER.log(
System.Logger.Level.DEBUG, LoggingSystem.format(loggingContext.subject()
, "Find entities with " + Arrays.toString(componentTypes))
);
}
switch (componentTypes.length) {
case 0:
return null;
case 1:
Node node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[0])));
return node == null ? null : node.copyOfLinkedNodes();
default:
Map<IndexKey, Node> currentCompositions = null;
// i = 0: node = A,B
// i = 1: node = C,D
// intersect(node1,node2) => nodeMap(size=0)
for (int i = 0; i < componentTypes.length; i++) {
node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentTypes[i])));
if (node == null) {
return null;
}
currentCompositions = currentCompositions == null ?
node.copyOfLinkedNodes() :
intersect(currentCompositions, node.linkedNodes)
;
}
return currentCompositions;
}
}Then, the method iterator in ResultSet does not judge the length sufficiently.
public Iterator<T> iterator() {
// if nodeMap.size() == 0 ?
return nodeMap != null ?
(nodeMap.size() > 1 ?
new IteratorWrapper<>(this, nodeMap.values().iterator()) :
compositionIterator(nodeMap.values().iterator().next().getComposition()))
:
new Iterator<>() {
@Override
public boolean hasNext() {
return false;
}
@Override
public T next() {
return null;
}
};
}Should it be changed to
public Iterator<T> iterator() {
// if nodeMap.size() == 0 ?
return nodeMap != null && nodeMap.size() > 0 ?
(nodeMap.size() > 1 ?
new IteratorWrapper<>(this, nodeMap.values().iterator()) :
compositionIterator(nodeMap.values().iterator().next().getComposition()))
:
new Iterator<>() {
@Override
public boolean hasNext() {
return false;
}
@Override
public T next() {
return null;
}
};
}@enricostara about withAlso method, do I understand correctly?
var w = Dominion.create();
w.findEntitiesWith(A.class, B.class)
// is equals
w.findEntitiesWith(A.class).withAlso(B.class)if my understand is right, I think the code below is problematic
public static void main(String[] args) {
var w = Dominion.create();
var s = w.createScheduler();
s.schedule(() -> {
var r = w.findEntitiesWith(A.class);
r.stream().forEach(rs -> System.out.println(rs.entity())); // print entity
r.withAlso(C.class).stream().forEach(rs -> System.out.println(rs.entity())); // also print entity
});
s.tickAtFixedRate(1);
w.createEntity(new A(), new B());
}I think the problem should be in mapWithAlso in CompositionRepository.java
public void mapWithAlso(Map<IndexKey, Node> nodeMap, Class<?>... componentTypes) {
if (componentTypes.length == 0) {
return;
}
for (Class<?> componentType : componentTypes) {
Node node = nodeCache.getNode(new IndexKey(classIndex.getIndex(componentType)));
if (node == null) {
continue; // this line
}
intersect(nodeMap, node.linkedNodes);
}
}I will check your test and the commit to fix the issue, if it's fine I will ask you to perform a Pull Request to integrate your contribution
enricostara
commented
1 year ago Hi @endison1986 even this issue should be fixed by getting the latest 0.8.0-SNAPSHOT
@enricostara I define three component, A,B,C
This is my test code:
it's fine.
I get NullPointerException
it's fine
I get ArrayIndexOutOfBoundsException
it's fine.
I get ArrayIndexOutOfBoundsException