Simsilica / SiO2

Base pack of useful reusable game code that can help bootstrap any JME game project.
BSD 3-Clause "New" or "Revised" License
25 stars 7 forks source link

Endless Loop in EntityContainer #4

Open MeFisto94 opened 7 years ago

MeFisto94 commented 7 years ago

It might be due to my special setup: I have something like class B extends A<Spatial>, whereas class A<T> extends EntityContainer<T>

The code in question is right in the constructor of the EntityContainer:

// Find out what the type parameter is       
        for( Type t = getClass().getGenericSuperclass(); t != null; ) {
            if( t instanceof ParameterizedType ) {
                ParameterizedType pt = (ParameterizedType)t;
                if( pt.getRawType() == EntityContainer.class ) {
                    parameter = (Class)pt.getActualTypeArguments()[0];
                    break;
                } 
            } else if( t instanceof Class ) {
                t = ((Class)t).getGenericSuperclass();
            } else {            
                t = null;
            }
        }

In my case t is instanceof ParameterizedType, but the RawType isn't EntityContainer.

I could get around this by adding t = ((Class)pt.getRawType()).getGenericSuperclass(); just before the first else-if, however that leads to a Cast Exception in parameter =.

For some reason, the rawType now is EntityContainer, however the ActualTypeArgument still is ParameterizedTypeImpl of class A<T>

MeFisto94 commented 7 years ago

So what I did as a workaround/fix here is:

// Find out what the type parameter is       
        for( Type t = getClass().getGenericSuperclass(); t != null; ) {
            if( t instanceof ParameterizedType ) {
                ParameterizedType pt = (ParameterizedType)t;
                if( pt.getRawType() instanceof Class && EntityContainer.class.isAssignableFrom(((Class)pt.getRawType())) ) {
//if( pt.getRawType() == EntityContainer.class ) {
                    if (pt.getActualTypeArguments()[0] instanceof Class) {
                        parameter = (Class)pt.getActualTypeArguments()[0];
                        break;
                    }
                }

                t = ((Class)pt.getRawType()).getGenericSuperclass();
            } else if( t instanceof Class ) {
                t = ((Class)t).getGenericSuperclass();
            } else {            
                t = null;
            }
        }

So what I changed are two things, the one is the already mentioned iteration just before the else if however that can be removed, since I discovered that further iterating until you are at EntityContainer leaves you with the ActualTypeArgument of T.

This is why the equality check has been changed to if( pt.getRawType() instanceof Class && EntityContainer.class.isAssignableFrom(((Class)pt.getRawType())) ) {. That way it works for multiple-level inheritances and catches the Generic at the top-level.

Another issue this solution has though is when you have multiple generic parameters like class A<X, Y> extends EntityContainer<Y>, for which there might not really be an easy solution apart from maybe getting rid of Reflection (granted such a use-case is very rare though)

pspeed42 commented 5 years ago

Looking at this, I think I understand... but it's going to be very hard for me to test it without a test case. I can create one but not in this round of patches. Leaving this open to look at again after this round of baselining.