-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Enable Godot reinstantiation on iOS for embedding in a single view #99705
base: master
Are you sure you want to change the base?
Conversation
The instance version will help us reinit what classes are currently init-and-forget
… for the _get_class_namev default (Object) implem
Feat/use godot as viewcontroller
You have a lot of |
The several TODOs in the code are all linked to the same root cause. This is the part we're unsure of, but couldn't find a better way to make it work. It could definitely be seen as a draft, but we're at our wits' end on this, so any feedback would be much appreciated. Not sure about the etiquette here, let me know if it should be converted to a draft. |
If it's ready to be reviewed and is in a form you would be comfortable merging it it should be marked as ready, if there are several issues still to resolve I'd suggest marking it as draft Especially seeing how there's parts of the feature you haven't worked out how to solve yet |
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.
Extra cleanup is probably fine, but I'm not sure what you are trying to do with static StringName
s. What's the purpose of reinitialization of StringName
s or class info? It (and most low level stiff like OS
) probably can be preserved and reused, you only need to reinitialize scene tree and view/view controller.
static StringName _class_name_static; \ | ||
if (unlikely(!_class_name_static)) { \ | ||
StringName::assign_static_unique_class_name(&_class_name_static, #m_class); \ | ||
static std::queue<StringName> _class_name_statics; \ |
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.
STL containers are not allowed, see https://docs.godotengine.org/en/stable/contributing/development/cpp_usage_guidelines.html#standard-template-library
The purpose of reinitializing |
We did originally try the For the stated goal of this PR, we ended up going with a more robust approach which is the libgodot/DisplayServerEmbedded approach patch that @kisg and his team developed and you can see in action here, and solves the iOS embedding problem, but also allows for not just embedding one view, but can support multiple views in the same screen (among other things): You can take it for a spin on iOS with both Swift or Godot-cpp with the samples here: https://github.com/migeran/libgodot_project The patches here won't hurt, but in our view, are neither sufficient or desirable for the intended goal. They did spot the challenging StringName issue, which is the ugliest part of the reset approach, but from memory, this is about a third of the static state that needs to be properly reset, there are plenty more globals and statics that need to be worked on. |
Partially adresses : godotengine/godot-proposals#1473.
The goal is to be able to run Godot from within a classically-made mobile app (think Snapchat Games). As underlined in the umbrella issue, several things will still be missing after this PR is merged :
We faced many issues related to instanciating Godot several times. We fixed all of them, but are unsure for some fixes ; specificially, we came up using a queue to store
StringName
references. We are iOS developers and we're humble about our C++ knowledge, but we're committed to make this work ; any feedback is therefore much appreciated.While the ability to embed Godot is functional with this PR, some manual adjustments are still required. An example implementation demonstrating how to embed Godot in a basic mobile app is available in our repository: here.