Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rcore] Configuration changes on Android are handled incorrectly #4254

Closed
4 tasks done
IllusionMan1212 opened this issue Aug 14, 2024 · 4 comments · Fixed by #4288
Closed
4 tasks done

[rcore] Configuration changes on Android are handled incorrectly #4254

IllusionMan1212 opened this issue Aug 14, 2024 · 4 comments · Fixed by #4288
Labels
platform: Android Android platform

Comments

@IllusionMan1212
Copy link
Contributor

  • I tested it on latest raylib version from master branch
  • I checked there is no similar issue already reported
  • I checked the documentation on the wiki
  • My code has no errors or misuse of raylib

Issue description

Whenever a configuration change happens (Orientation change, screen size change, a USB keyboard is plugged in, etc..) the Android OS has to destroy and recreate the activity from scratch. When using a NativeActivity with the glue code provided in the NDK, android_app.destroyRequested is set to 1.

The way raylib currently handles activity recreation is to ignore destroyRequested, which ends up with the app not responding due to the system sending PAUSE, STOP, and DESTROY commands. Raylib happily destroys the surface and GL context; however, the app thread is never terminated, which is what the system expects to happen so it can create the activity again.

One solution is to have the app opt to handle certain configuration changes itself and instruct the system not to recreate the activity by declaring certain values in the android:configChanges manifest attribute.
This approach works well for handling orientation changes, but it might not work for other config changes that are either not handled or just refuse to work (e.g. USB keyboard always recreates the activity even if keyboard is declared in android:configChanges).

The proper way to solve this is to correctly handle destroyRequested by letting android_main return to its caller and allowing the system to recreate the NativeActivity and call android_main again.
Of course, calling android_main more than once means raylib will have to reinitialize global state to 0 and somehow keep track of game data that needs to be restored by saving it to android_app.savedState when the SAVE_STATE command is issued.

Here's a small diff that fixes the hangs but doesn't do any game state saving.

diff --git a/src/platforms/rcore_android.c b/src/platforms/rcore_android.c
index 68ae979e..124a33c8 100644
--- a/src/platforms/rcore_android.c
+++ b/src/platforms/rcore_android.c
@@ -696,7 +696,7 @@ void PollInputEvents(void)
         // NOTE: Never close window, native activity is controlled by the system!
         if (platform.app->destroyRequested != 0)
         {
-            //CORE.Window.shouldClose = true;
+            CORE.Window.shouldClose = true;
             //ANativeActivity_finish(platform.app->activity);
         }
     }
@@ -811,6 +811,11 @@ void ClosePlatform(void)
         eglTerminate(platform.device);
         platform.device = EGL_NO_DISPLAY;
     }
+
+    if (platform.app->destroyRequested != 0) {
+        CORE = (CoreData){0};
+        platform = (PlatformData){0};
+    }
 }
 
 // Initialize display device and framebuffer

Environment

Platform: Android
Operating System: Android 11
OpenGL Version: GLES 3.2 (Tested app is using GLES 2.0)
GPU: N/A

Issue Screenshot

N/A

Code Example

Any example from examples/ will do, I tested with shapes/shapes_bouncing_ball

@raysan5
Copy link
Owner

raysan5 commented Aug 16, 2024

@IllusionMan1212 Actually, it's been some years not using Android backend; feel free to send a PR with those changes. About the state saving, I imagine it should be managed at user side...

@IllusionMan1212
Copy link
Contributor Author

About the state saving, I imagine it should be managed at user side...

I think the only way for this to work is to have the user store their game state globally and not reinitialize it when their main function is called again, although I'm not sure what implications this will have on how the system manages its resources.
The Android way is to save any needed state in android_app.savedState when APP_CMD_SAVE_STATE is received and restore the state on activity recreation from the android_app that's passed to android_main.

I can experiment with both approaches and provide patches and thoughts.

@IllusionMan1212
Copy link
Contributor Author

So here's something quick that works for savedState

diff --git a/src/platforms/rcore_android.c b/src/platforms/rcore_android.c
index 68ae979e..b29479eb 100644
--- a/src/platforms/rcore_android.c
+++ b/src/platforms/rcore_android.c
@@ -45,6 +45,7 @@
 *     3. This notice may not be removed or altered from any source distribution.
 *
 **********************************************************************************************/
+#include <stdlib.h>
 
 #include <android_native_app_glue.h>    // Required for: android_app struct and activity management
 #include <android/window.h>             // Required for: AWINDOW_FLAG_FULLSCREEN definition and others
@@ -56,6 +57,12 @@
 //----------------------------------------------------------------------------------
 // Types and Structures Definition
 //----------------------------------------------------------------------------------
+typedef void (*SaveGameStateCallback)(void **savedState, size_t *savedStateSize);
+typedef void (*RestoreGameStateCallback)(void **savedState);
+
+SaveGameStateCallback gameStateSave;
+RestoreGameStateCallback gameStateRestore;
+
 typedef struct {
     // Application data
     struct android_app *app;            // Android activity
@@ -275,6 +282,10 @@ void android_main(struct android_app *app)
     char arg0[] = "raylib";     // NOTE: argv[] are mutable
     platform.app = app;
 
+    if (app->savedState != NULL) {
+        gameStateRestore(&app->savedState);
+    }
+
     // NOTE: Return from main is ignored
     (void)main(1, (char *[]) { arg0, NULL });
 
@@ -693,11 +704,9 @@ void PollInputEvents(void)
         // Process this event
         if (platform.source != NULL) platform.source->process(platform.app, platform.source);
 
-        // NOTE: Never close window, native activity is controlled by the system!
         if (platform.app->destroyRequested != 0)
         {
-            //CORE.Window.shouldClose = true;
-            //ANativeActivity_finish(platform.app->activity);
+            CORE.Window.shouldClose = true;
         }
     }
 }
@@ -780,7 +789,7 @@ int InitPlatform(void)
             // Process this event
             if (platform.source != NULL) platform.source->process(platform.app, platform.source);
 
-            // NOTE: Never close window, native activity is controlled by the system!
+            // NOTE: It's highly likely destroyRequested will never be non-zero at the start of the activity lifecycle.
             //if (platform.app->destroyRequested != 0) CORE.Window.shouldClose = true;
         }
     }
@@ -811,6 +820,11 @@ void ClosePlatform(void)
         eglTerminate(platform.device);
         platform.device = EGL_NO_DISPLAY;
     }
+
+    if (platform.app->destroyRequested != 0) {
+        CORE = (CoreData){0};
+        platform = (PlatformData){0};
+    }
 }
 
 // Initialize display device and framebuffer
@@ -1072,7 +1086,10 @@ static void AndroidCommandCallback(struct android_app *app, int32_t cmd)
             // this means that the user has already called 'CloseWindow()'
 
         } break;
-        case APP_CMD_SAVE_STATE: break;
+        case APP_CMD_SAVE_STATE:
+        {
+            gameStateSave(&app->savedState, &app->savedStateSize);
+        } break;
         case APP_CMD_STOP: break;
         case APP_CMD_DESTROY: break;
         case APP_CMD_CONFIG_CHANGED:
@@ -1086,6 +1103,11 @@ static void AndroidCommandCallback(struct android_app *app, int32_t cmd)
     }
 }
 
+void SetGameStateFuncs(SaveGameStateCallback saveCallback, RestoreGameStateCallback restoreCallback) {
+    gameStateSave = saveCallback;
+    gameStateRestore = restoreCallback;
+}
+
 // ANDROID: Map Android gamepad button to raylib gamepad button
 static GamepadButton AndroidTranslateGamepadButton(int button)
 {

And the game code for bouncing ball will look like this

#include "raylib.h"
#include <stdlib.h>

typedef struct {
    Vector2 ballPosition;
    Vector2 ballSpeed;
    bool hasStarted;
} State;

State gameState;

void onSaveState(void **savedState, size_t *savedStateSize) {
    // NOTE: The system will free this memory for us.
    *savedState = malloc(sizeof(State));
    **(State**)savedState = gameState;
    *savedStateSize = sizeof(State);
}

void onRestoreState(void **savedState) {
    gameState = **(State**)savedState;
}

int main(void)
{
    const int screenWidth = 800;
    const int screenHeight = 450;

    SetConfigFlags(FLAG_MSAA_4X_HINT);
    InitWindow(screenWidth, screenHeight, "raylib [shapes] example - bouncing ball");

    SetGameStateFuncs(onSaveState, onRestoreState);

    if (!gameState.hasStarted) {
        gameState.ballPosition = (Vector2){ GetScreenWidth()/2.0f, GetScreenHeight()/2.0f };
        gameState.ballSpeed = (Vector2){ 5.0f, 4.0f };

        gameState.hasStarted = true;
    }
    
    // The rest of the example code
}

Here's how the user code will look like if the state saving is managed by the user.

#include "raylib.h"

typedef struct {
    Vector2 ballPosition;
    Vector2 ballSpeed;
} State;

State *gameState = NULL;

int main(void)
{
    const int screenWidth = 800;
    const int screenHeight = 450;

    SetConfigFlags(FLAG_MSAA_4X_HINT);
    InitWindow(screenWidth, screenHeight, "raylib [shapes] example - bouncing ball");

    if (gameState == NULL) {
        gameState = (State*)malloc(sizeof(State));

        gameState->ballPosition = (Vector2){ GetScreenWidth()/2.0f, GetScreenHeight()/2.0f };
        gameState->ballSpeed = (Vector2){ 5.0f, 4.0f };
    }
    
    // The rest of the example code
}

Both approaches work and are very similar in how the state is stored. I don't know how to feel about "forcing" the user to store their game state globally. Maybe you have better ideas for how this should be handled.

@raysan5
Copy link
Owner

raysan5 commented Aug 24, 2024

@IllusionMan1212 Proposed additions to rcore_android.c look ok to me to address this issue, I'm not exposing those functions on raylib.h for now but maybe in a future.

Feel free to send a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Android Android platform
Projects
None yet
2 participants