Fix create_instance in android GodotApp so non-editor apps can restart #101050

jamie-pate · 2025-01-02T20:46:15Z

Enables OS.create_instance(args) and OS.set_restart_on_exit(true, args) on android.

Borrowed the logic from the editor, so it completely restarts the process so you can pass --rendering-method, --rendering-driver to switch between forward_plus, mobile, gl_compatibility etc on an exported app.

Related:
#31541
godotengine/godot-proposals#6423

jamie-pate · 2025-01-02T22:28:11Z

Tested on Pixel 8 and Meta Quest 3

allows switching rendering methods by calling get_tree().quit() combined with:

OS.set_restart_on_exit("--rendering-method gl_compatibility")
OS.set_restart_on_exit("--rendering-method mobile")

godot_app_android_restart_test.zip

platform/android/java/app/src/com/godot/game/GodotApp.java

platform/android/java/lib/src/org/godotengine/godot/Godot.kt

platform/android/java/app/src/com/godot/game/GodotApp.java

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

platform/android/java/app/src/com/godot/game/GodotApp.java

m4gr3d · 2025-01-04T03:42:46Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-		}
-		if (BuildConfig.BUILD_TYPE == "dev") {
-			commandLineParams.add("--benchmark")
+		val args = if (BuildConfig.BUILD_TYPE == "dev") {


The new variable should have a different name otherwise it shadows the function parameter, and may lead to subtle bugs / mistakes when this method is updated in the future.

IMO that's a feature, not a bug.. if you have multiple variables around that's when you might choose the wrong variable
https://discuss.kotlinlang.org/t/rust-style-variable-shadowing/16338

if there's only one 'args' then you can't use the 'wrong' one. I want to 'destroy' the old 'args' value

That's a fair argument.. I don't have a preference one way or another but since it seems to match the use-case here, it can remain as is. Note though that this would not work in the GodotXRGame use-case since we're updating the arguments several times based on separate constraints.

It makes sense to me to add a bunch of stuff to a list, then call toTypedArray() to pass it on in that case (which is how GodotXRGame.updateCommandLineParams() works in this change)

Really this method should be named setCommandLineParams() since it's only ever called once and always sets the entire array/list at once and each level only really calls it once to set it's arguments and then passes them to super().

IMO The whole dance here is a bit much. To simplify I would eliminate this method and just make it a protected List<String> and let each level add it's own args. The consumer could call public getCommandLine(): Array<String> to get the result after the end.

Paranoia: this may be too simple in some case that doesn't exist yet if arguments needed to be strictly ordered .. but it's a protected member so you'd just have to refactor it then, and currently it doesn't look like they do need to be strictly ordered.

If setCommandLineParams() threw when it's too late then that would make sense though.

Right now I don't want to do any more refactoring in this change though, already have too much whiplash :D

m4gr3d · 2025-01-04T03:48:09Z

platform/android/java/lib/src/org/godotengine/godot/GodotActivity.kt

+	@CallSuper
+	protected open fun getNewGodotInstanceIntent(args: Array<String>): Intent {
+		return Intent().setComponent(ComponentName(this, javaClass.name))
+			.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+			.putExtra(EXTRA_COMMAND_LINE_PARAMS, args)
+	}
+
+	override fun onNewGodotInstanceRequested(args: Array<String>): Int {
+		Log.d(TAG, "Restarting with parameters ${args.contentToString()}")
+		triggerRebirth(null, getNewGodotInstanceIntent(args))
+		// fake 'process' id returned by create_instance() etc
+		return DEFAULT_WINDOW_ID;
+	}


Adding the triggerRebirth function is a good idea! However, I don't see similar benefits with getNewGodotInstanceIntent and onNewGodotInstanceRequested. The latter especially is different enough between the two classes that there's not much benefit in trying to extract common functionality, so I'd just leave it as is.

This functionality is needed here since I removed it from GodotApp to avoid kotlin/java interop..

The default behavior (here) is used by GodotApp, BaseGodotEditor doesn't call super.onNewGodotInstanceRequested(...) since the only common functionality is triggerRebirth()

For getNewGodotInstanceIntent():
I feel that addFlags() and putExtra() are enough common functionality between GodotActivity and BaseGodotEditor, since they both need those basic intent options configured to function. (BaseGodotEditor just calls intent.setComponent() again to override the classname.

I thought about renaming it to getNewGodotEditorInstanceIntent(...) just to avoid calling retrieveEditorWindowInfo() twice...

This functionality is needed here since I removed it from GodotApp to avoid kotlin/java interop..

What's the interop issue you're referring to?

For getNewGodotInstanceIntent():
I feel that addFlags() and putExtra() are enough common functionality between GodotActivity and BaseGodotEditor, since they both need those basic intent options configured to function. (BaseGodotEditor just calls intent.setComponent() again to override the classname.

Let's remove this one.. creating an intent is simple enough that we don't really need an helper method for it. In addition, each subclass has different requirements on how they need to configure their intent and the arguments they're passing to it.

This functionality is needed here since I removed it from GodotApp to avoid kotlin/java interop..

What's the interop issue you're referring to?

I interop works fine but the impedance mismatch is higher than expected to call across the two. Specifically this
getEXTRA_COMMAND_LINE_PARAMS() quirk caused me to decide that It's better to just write as much of this change in GodotActivity.kt to avoid crossing between java and kotlin.

Let's remove this one..

Done.

platform/android/java/editor/src/horizonos/java/org/godotengine/editor/GodotXRGame.kt

llama-nl · 2025-01-09T03:44:28Z

I tested the Android Editor with this project:

godot_app_android_restart_test.zip

It restart the Editor when tried to change the renderer. And the PIP button also apear in editor.

az_recorder_20250109_094022_edited.mp4

jamie-pate · 2025-01-09T16:44:46Z

It restart the Editor when tried to change the renderer.

This would be a separate issue.

When running from the Editor on Android it isn't really running a separate process.
You can't really restart the process while debugging on desktop either.
you need to export an app and install the apk separate from the Editor to test this fix

llama-nl · 2025-01-10T02:58:29Z

When running from the Editor on Android it isn't really running a separate process.

You can't really restart the process while debugging on desktop either.

I know that. But I was expecting a warning. No behavior like this.

when calling those function you could just show a wearing and do noting. For better debuning.

m4gr3d

Almost ready for approval!

m4gr3d · 2025-01-13T02:38:30Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-		}
-		if (BuildConfig.BUILD_TYPE == "dev") {
-			commandLineParams.add("--benchmark")
+		val args = if (BuildConfig.BUILD_TYPE == "dev") {


That's a fair argument.. I don't have a preference one way or another but since it seems to match the use-case here, it can remain as is. Note though that this would not work in the GodotXRGame use-case since we're updating the arguments several times based on separate constraints.

m4gr3d · 2025-01-13T02:38:57Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-		if (BuildConfig.BUILD_TYPE == "dev") {
-			commandLineParams.add("--benchmark")
+		val args = if (BuildConfig.BUILD_TYPE == "dev") {
+			arrayOf(*args, "--benchmark")


nit: here you can just return args + "--benchmark"

Perfect, loving kotlin more the more I learn of the syntax!

m4gr3d · 2025-01-13T02:46:24Z

platform/android/java/lib/src/org/godotengine/godot/GodotActivity.kt

+	@CallSuper
+	protected open fun getNewGodotInstanceIntent(args: Array<String>): Intent {
+		return Intent().setComponent(ComponentName(this, javaClass.name))
+			.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+			.putExtra(EXTRA_COMMAND_LINE_PARAMS, args)
+	}
+
+	override fun onNewGodotInstanceRequested(args: Array<String>): Int {
+		Log.d(TAG, "Restarting with parameters ${args.contentToString()}")
+		triggerRebirth(null, getNewGodotInstanceIntent(args))
+		// fake 'process' id returned by create_instance() etc
+		return DEFAULT_WINDOW_ID;
+	}


This functionality is needed here since I removed it from GodotApp to avoid kotlin/java interop..

What's the interop issue you're referring to?

For getNewGodotInstanceIntent():
I feel that addFlags() and putExtra() are enough common functionality between GodotActivity and BaseGodotEditor, since they both need those basic intent options configured to function. (BaseGodotEditor just calls intent.setComponent() again to override the classname.

Let's remove this one.. creating an intent is simple enough that we don't really need an helper method for it. In addition, each subclass has different requirements on how they need to configure their intent and the arguments they're passing to it.

Enables OS.create_instance(args) and OS.set_restart_on_exit(true, args) on android. Borrowed the logic from the editor, so it completely restarts the process so you can pass --rendering-method, --rendering-driver to switch between forward_plus, mobile, gl_compatibility etc on an exported app. Related: godotengine/godot-proposals#6423

jamie-pate

🙏

jamie-pate · 2025-01-19T11:01:38Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-		}
-		if (BuildConfig.BUILD_TYPE == "dev") {
-			commandLineParams.add("--benchmark")
+		val args = if (BuildConfig.BUILD_TYPE == "dev") {


It makes sense to me to add a bunch of stuff to a list, then call toTypedArray() to pass it on in that case (which is how GodotXRGame.updateCommandLineParams() works in this change)

Really this method should be named setCommandLineParams() since it's only ever called once and always sets the entire array/list at once and each level only really calls it once to set it's arguments and then passes them to super().

IMO The whole dance here is a bit much. To simplify I would eliminate this method and just make it a protected List<String> and let each level add it's own args. The consumer could call public getCommandLine(): Array<String> to get the result after the end.

Paranoia: this may be too simple in some case that doesn't exist yet if arguments needed to be strictly ordered .. but it's a protected member so you'd just have to refactor it then, and currently it doesn't look like they do need to be strictly ordered.

If setCommandLineParams() threw when it's too late then that would make sense though.

Right now I don't want to do any more refactoring in this change though, already have too much whiplash :D

jamie-pate · 2025-01-19T11:01:41Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-		if (BuildConfig.BUILD_TYPE == "dev") {
-			commandLineParams.add("--benchmark")
+		val args = if (BuildConfig.BUILD_TYPE == "dev") {
+			arrayOf(*args, "--benchmark")


Perfect, loving kotlin more the more I learn of the syntax!

jamie-pate · 2025-01-19T11:11:37Z

platform/android/java/lib/src/org/godotengine/godot/GodotActivity.kt

+	@CallSuper
+	protected open fun getNewGodotInstanceIntent(args: Array<String>): Intent {
+		return Intent().setComponent(ComponentName(this, javaClass.name))
+			.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+			.putExtra(EXTRA_COMMAND_LINE_PARAMS, args)
+	}
+
+	override fun onNewGodotInstanceRequested(args: Array<String>): Int {
+		Log.d(TAG, "Restarting with parameters ${args.contentToString()}")
+		triggerRebirth(null, getNewGodotInstanceIntent(args))
+		// fake 'process' id returned by create_instance() etc
+		return DEFAULT_WINDOW_ID;
+	}


This functionality is needed here since I removed it from GodotApp to avoid kotlin/java interop..

What's the interop issue you're referring to?

I interop works fine but the impedance mismatch is higher than expected to call across the two. Specifically this
getEXTRA_COMMAND_LINE_PARAMS() quirk caused me to decide that It's better to just write as much of this change in GodotActivity.kt to avoid crossing between java and kotlin.

Let's remove this one..

Done.

m4gr3d

Looks good! Thanks for the fix!

m4gr3d · 2025-01-21T04:38:10Z

platform/android/java/editor/src/main/java/org/godotengine/editor/BaseGodotEditor.kt

-	final override fun getCommandLine() = commandLineParams
-
-	protected fun retrieveEditorWindowInfo(args: Array<String>): EditorWindowInfo {
+	protected open fun retrieveEditorWindowInfo(args: Array<String>): EditorWindowInfo {


nit: this change can be reverted

jamie-pate · 2025-01-21T19:41:47Z

@syntaxerror247 i think this is ready :D

syntaxerror247

Looks good! Thanks!

Repiteo · 2025-01-22T22:37:38Z

Thanks!

jamie-pate requested a review from a team as a code owner January 2, 2025 20:46

jamie-pate mentioned this pull request Jan 2, 2025

Implement changing RenderingDevice at runtime godotengine/godot-proposals#6423

Open

jamie-pate force-pushed the fix_android_godot_app_create_instance branch 2 times, most recently from 751b5cc to 826fa32 Compare January 2, 2025 21:33

Calinou added bug platform:android topic:porting labels Jan 2, 2025

Calinou added this to the 4.4 milestone Jan 2, 2025

Calinou added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jan 2, 2025

m4gr3d requested changes Jan 3, 2025

View reviewed changes

jamie-pate force-pushed the fix_android_godot_app_create_instance branch 3 times, most recently from abcdad7 to 9dc9609 Compare January 3, 2025 03:06

m4gr3d requested changes Jan 3, 2025

View reviewed changes

jamie-pate force-pushed the fix_android_godot_app_create_instance branch 5 times, most recently from 66e2376 to b182320 Compare January 3, 2025 23:05

jamie-pate requested a review from m4gr3d January 4, 2025 00:31

m4gr3d requested a review from syntaxerror247 January 4, 2025 03:36

m4gr3d requested changes Jan 4, 2025

View reviewed changes

jamie-pate force-pushed the fix_android_godot_app_create_instance branch from b182320 to 298bfe3 Compare January 5, 2025 00:30

jamie-pate requested a review from m4gr3d January 7, 2025 01:16

m4gr3d requested changes Jan 13, 2025

View reviewed changes

jamie-pate force-pushed the fix_android_godot_app_create_instance branch from 298bfe3 to 605b970 Compare January 19, 2025 11:24

jamie-pate commented Jan 19, 2025

View reviewed changes

jamie-pate requested a review from m4gr3d January 19, 2025 11:47

m4gr3d approved these changes Jan 21, 2025

View reviewed changes

syntaxerror247 approved these changes Jan 22, 2025

View reviewed changes

syntaxerror247 added enhancement bug and removed cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release bug enhancement labels Jan 22, 2025

Repiteo merged commit b0655dc into godotengine:master Jan 22, 2025
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix create_instance in android GodotApp so non-editor apps can restart #101050

Fix create_instance in android GodotApp so non-editor apps can restart #101050

jamie-pate commented Jan 2, 2025

jamie-pate commented Jan 2, 2025 •

edited

Loading

m4gr3d Jan 4, 2025

jamie-pate Jan 5, 2025

m4gr3d Jan 13, 2025

jamie-pate Jan 19, 2025

m4gr3d Jan 4, 2025

jamie-pate Jan 5, 2025 •

edited

Loading

m4gr3d Jan 13, 2025

jamie-pate Jan 19, 2025 •

edited

Loading

llama-nl commented Jan 9, 2025 •

edited

Loading

jamie-pate commented Jan 9, 2025 •

edited

Loading

llama-nl commented Jan 10, 2025

m4gr3d left a comment

m4gr3d Jan 13, 2025

m4gr3d Jan 13, 2025

jamie-pate Jan 19, 2025

m4gr3d Jan 13, 2025

jamie-pate left a comment

jamie-pate Jan 19, 2025

jamie-pate Jan 19, 2025

jamie-pate Jan 19, 2025 •

edited

Loading

m4gr3d left a comment

m4gr3d Jan 21, 2025

jamie-pate commented Jan 21, 2025

syntaxerror247 left a comment

Repiteo commented Jan 22, 2025

Fix create_instance in android GodotApp so non-editor apps can restart #101050

Fix create_instance in android GodotApp so non-editor apps can restart #101050

Conversation

jamie-pate commented Jan 2, 2025

jamie-pate commented Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie-pate Jan 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie-pate Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

llama-nl commented Jan 9, 2025 • edited Loading

jamie-pate commented Jan 9, 2025 • edited Loading

llama-nl commented Jan 10, 2025

m4gr3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie-pate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie-pate Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

m4gr3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie-pate commented Jan 21, 2025

syntaxerror247 left a comment

Choose a reason for hiding this comment

Repiteo commented Jan 22, 2025

jamie-pate commented Jan 2, 2025 •

edited

Loading

jamie-pate Jan 5, 2025 •

edited

Loading

jamie-pate Jan 19, 2025 •

edited

Loading

llama-nl commented Jan 9, 2025 •

edited

Loading

jamie-pate commented Jan 9, 2025 •

edited

Loading

jamie-pate Jan 19, 2025 •

edited

Loading