-
-
Notifications
You must be signed in to change notification settings - Fork 22.7k
Add support for running hybrid apps from the XR editor #103972
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
Conversation
62d2e1b
to
538830b
Compare
6171a82
to
a3a7f8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested on Quest 3, and this is pretty impressive! When switching back to panel app, it opens the app embedded and the debugger is connected.
Skimming the code, it all looks good! I only had a question about a change outside of the Android platform
if (dbg->is_session_active()) { | ||
dbg->_stop_and_notify(); | ||
} | ||
dbg->stop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change necessary? It would seem to affect all platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bug I found while testing, which I think affect all platforms.
When EditorDebuggerNode::stop(...)
is invoked, if the debugging session doesn't have an active session, it won't be stopped alongside EditorDebuggerNode
, so it'll continue processing
because set_process(false)
is only invoked in ScriptEditorDebugger::stop()
.
This'll cause:
_stop_and_notify()
ingodot/editor/debugger/script_editor_debugger.cpp
Line 1115 in 51b0379
_stop_and_notify(); - which will fire a
stopped
signal - which will cause
EditorDebuggerNode::_debugger_stopped
to be invoked because ofnode->connect("stopped", callable_mp(this, &EditorDebuggerNode::_debugger_stopped).bind(id)); - which will trigger
notify_all_debug_sessions_exited
- which invokes
project_run_bar->stop_playing()
- which in turn invokes
EditorDebuggerNode::get_singleton()->stop()
again
In most cases, it's not an issue because EditorRunBar::stop_playing()
abort early when nothing is playing, but it becomes an issue when a project is playing after the first call to EditorDebuggerNode::stop()
, as the call in (5)
will cause the running project to be stopped as a consequence of the described flow.
So the fix is to always invoke ScriptEditorDebugger::stop()
when EditorDebuggerNode::stop()
is invoked in order to also stop processing
for ScriptEditorDebugger
. _stop_and_notify()
is not used to avoid invoking EditorDebuggerNode::stop()
again when the stopped
signal is fired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. I walked through the code, and the chain of events you describe makes sense. However, are you sure there aren't any cases where something is expecting EditorDebuggerNode::stop()
to lead to ScriptEditorDebugger
's "stopped"
signal being emitted, and now it won't be?
This change should probably be reviewed by someone who understands the debugger really well. Perhaps @Faless? I don't know who else knows this code? cc @akien-mga
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested switching to _stop_and_notify()
and it seems to work as expected, so I've restored that call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that seems like a less potentially impactful change to me
e4cb5e6
to
1b1905f
Compare
1b1905f
to
09f5be7
Compare
Thanks! |
As the title mentions, this PR adds support for developing and debugging hybrid apps directly from the XR editor.
For context, Hybrid Apps are applications capable of swapping between an immersive 3D mode and a 2D panel mode at runtime.
The Godot XR editor itself is an hybrid app, and with this update, it gains and provides the ability to developers to create hybrid apps directly from a standalone XR device.
Recording using the Hybrid App sample project from a Quest 3 device:
com.oculus.metacam-20250610-214441-0.mp4
Note: Hybrid apps require v4 of the Godot OpenXR Vendor plugin