jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.83k stars 1.12k forks source link

Starting an Android App on Nougat fails because of NPE #1293

Open MeFisto94 opened 4 years ago

MeFisto94 commented 4 years ago
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'com.jme3.system.JmeContext com.jme3.app.LegacyApplication.getContext()' on a null object reference
        at com.jme3.app.AndroidHarness.onCreate(AndroidHarness.java:255)
        at android.app.Activity.performCreate(Activity.java:6679)
MeFisto94 commented 4 years ago

Seems this issue is related to #1292 in a strange way: AndroidHarness#onCreate tries to load AndroidHarness#appClass with Reflection (It is unclear to me why not specifying a class at least, so the user could handle the resolution).

Anyway the class is protected String appClass = "jme3test.android.Test";, which to me makes zero sense in a general purpose AndroidHarness. One of these classes might remotely help, but they still don't have Test directly etc pp. To me that sounds fishy, specifically since we expect it to be a subclass of LegacyApplication. Would it even make sense to let users specify their specific Application class? I mean they can't extend that class without a major overriding of AndroidHarness, but I guess a very very optional Class<? extends AndroidApplication> getApplicationClass() would be much better.

MeFisto94 commented 4 years ago

Moreover failing of app creation doesn't terminate everything but proceeds until the NPE in the next line. Edit: Specifying SimpleApplication as appClass yields: W/System.err: java.lang.InstantiationException: java.lang.Class<com.jme3.app.SimpleApplication> cannot be instantiated Maybe this is something android related?

Edit2: I am stupid, SimpleApplication is an abstract class, so it's probably more important to supply a default application, specifically because extending shouldn't give users anything in the future (instead, they should just use AppStates etc)

MeFisto94 commented 4 years ago

So basically we now have: #1292 as a discussion for the default Application and this issue as a discussion on how to access/set the class instance.

I think we all agree that a String is the weakest way of setting a Class<? extends Application> for various reasons.

I've also thought about the above idea of using getApplicationClass() and I figured the best would be to make AndroidHarness generic, i.e.: AndroidHarness<T extends Application>.

That way one could make MyAndroidActivity extends AndroidHarness<MyApplication> or also MyAndroidActivity extends AndroidHarness<SimpleApplication>*. Is there a downside to using generics in that way, @pspeed42? My main reason for this is, that with Generics, we can have a T getApplication, which won't be Application so you have to always cast the results when trying to access stuff.

* this obviously doesn't work due to SimpleApplication being abstract, but we could still have an Android Application or something (see #1292 )

pspeed42 commented 4 years ago

But if you don't want to have to cast then you can always override the method yourself to return your own type. I know it's a little more extra work but generics get weird sometimes so they are not always "free". There comes a follow-on effect for all the code that now has to deal with AndroidHarness<?> or have lots of suppress warnings here and there.

pavly-gerges commented 3 years ago
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'com.jme3.system.JmeContext com.jme3.app.LegacyApplication.getContext()' on a null object reference
        at com.jme3.app.AndroidHarness.onCreate(AndroidHarness.java:255)
        at android.app.Activity.performCreate(Activity.java:6679)

Hi @MeFisto94 can I know what did you do exactly , to reproduce this exception , if you can post the whole stack, thanks 👍.

pavly-gerges commented 3 years ago

Seems this issue is related to #1292 in a strange way: AndroidHarness#onCreate tries to load AndroidHarness#appClass with Reflection (It is unclear to me why not specifying a class at least, so the user could handle the resolution).

Anyway the class is protected String appClass = "jme3test.android.Test";, which to me makes zero sense in a general purpose AndroidHarness. One of these classes might remotely help, but they still don't have Test directly etc pp. To me that sounds fishy, specifically since we expect it to be a subclass of LegacyApplication. Would it even make sense to let users specify their specific Application class? I mean they can't extend that class without a major overriding of AndroidHarness, but I guess a very very optional Class<? extends AndroidApplication> getApplicationClass() would be much better.

What about this factory method abstract Class<? extends Application> getInstance(); :

package com.myGame.JMESurfaceViewExampleActivity;

import com.jme3.app.Application;
import com.myGame.JmEGamePadExample.JmeGame;
import com.scrappers.superiorExtendedEngine.jmeSurfaceView.compat.CompatHarness;

/**
 * An Android CompatHarness Migration test.
 * @author pavl_g.
 */
public class TestCompatHarness extends CompatHarness {

    @Override
    protected void preInitializeGlConfig() {
        //TODO define your egl, display and LayoutManager config here

        EGLConfig.setEglSurfaceDelay(100);
        EGLConfig.setEglAlphaBits(10);
        EGLConfig.setEglDepthBits(16);
        EGLConfig.setEglStencilBits(10);
        EGLConfig.setFrameRate(-1);
        Display.switchToGameMode(this);

        LayoutManager.setLayoutManager(LayoutManager.No_Layout);
        LayoutManager.setLayoutManager(LayoutManager.Relative_Layout);
    }

    @Override
    protected Class<? extends Application> getInstance() {
        //TODO return your game class here
        return JmeGame.class;
    }

    @Override
    protected void onExceptionThrown(Throwable throwable) {
        //TODO catch and deal with exceptions/errors here
    }

    @Override
    protected void onStartRenderer(Application app) {
        //TODO do something when the renderer starts
    }

    @Override
    protected void onRendererCompletion(Application app) {
        //TODO do something when the renderer completes
    }

    @Override
    protected void onStopRenderer(Application app) {
        //TODO do something when the renderer stops
    }

    @Override
    protected void onPauseRenderer(Application app) {
        //TODO do something when the renderer is paused
    }

    @Override
    protected void onLostFocus(Application app) {
        //TODO do something when jme loses focus
    }

    @Override
    protected void onGainedFocus(Application app) {
        //TODO do something when jme gains focus
    }
}

The new CompatHarness uses androidx compatibility API instead of plain android.

MeFisto94 commented 2 years ago

Hi @MeFisto94 can I know what did you do exactly , to reproduce this exception , if you can post the whole stack, thanks 👍. FWIW that is the full stack trace: https://github.com/jMonkeyEngine/jmonkeyengine/blob/29d8134eaf263855e66aed485f444df2dcb648dd/jme3-android/src/main/java/com/jme3/app/AndroidHarness.java#L254

Here app can simply be null due to a string issue, it would be a lot better to only have this issue whenever the class cannot be loaded from the classpath (if that is even possible without ClassNotFoundErrors)