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

Re-architecture of the Godot Android plugin. #33682

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Nov 17, 2019

This PR revisits the Godot Android plugins' architecture.
This is done by leveraging the Android library file format. A Godot Android plugin is now defined as follow:

A Godot Android plugin is a regular Android library packaged as an aar archive file with the following caveats:

  • The library must have a dependency on the Godot Android library (godot-lib.aar). A stable version is made available for each release.

  • The library must include a <meta-data> tag in its manifest file setup as follow:
    <meta-data android:name="org.godotengine.plugin.v1.[PluginName]" android:value="[plugin.init.ClassFullName]" />
    Where:

  • PluginName is the name of the plugin.

  • plugin.init.ClassFullName is the full name (package + class name) of the plugin class extending GodotPlugin.

At runtime, Godot (via GodotPluginRegistry) will parse the Android manifest, and retrieve all components with the specified <meta-data> tag. It'll then use reflection to instantiate and load the plugin class defined by plugin.init.ClassFullName. From there, the plugins follow the same lifecycle as with the previous architecture.

A nice side-effect of this new architecture is that we can remove all the gradle build and android manifest custom update logic, since we can just rely on the Android library build and manifest merge mechanism.

Documentation update: godotengine/godot-docs#2979

Note:

  • This is a breaking / non-backward compatible change with the previous Godot Android plugin architecture.
  • Just like the previous architecture, this requires the use of custom builds.
  • This new architecture may be helpful for PR [WIP] Adding ARCore #26221, as the ARCore build dependencies no longer need to be included in Godot core. This would however limit ARCore to custom builds only.
  • The updated architecture simplifies the process of bundling Android gdnative libraries with Android java libraries. See the updated documentation for details.

Example implementation:

@m4gr3d m4gr3d requested a review from a team as a code owner November 17, 2019 19:06
@m4gr3d m4gr3d requested a review from a team November 17, 2019 19:07
@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 2 times, most recently from 49ec3a1 to 6829764 Compare November 17, 2019 19:40
@Chaosus Chaosus added this to the 4.0 milestone Nov 18, 2019
@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 2 times, most recently from fc373c4 to 0da2da7 Compare November 25, 2019 21:07
@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 3 times, most recently from f0547e6 to 382a2e9 Compare December 11, 2019 21:39
@m4gr3d m4gr3d changed the title WIP: Re-architecture of the Godot Android plugin. Re-architecture of the Godot Android plugin. Dec 12, 2019
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Dec 12, 2019

@akien-mga @BastiaanOlij @godotengine/android PR is ready for feedback!
PR for updated documentation is godotengine/godot-docs#2979

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this works as advertised I think its a great step forward. But not being super experienced with Android I'll have to take your word for it :)

This may solve the deploy issues for ARCore as well, might need to think of exposing a part of the camera server through GDNative first.

How is this working in conjunction with GDNatives config? Right now GDNative will do the work in making sure the plugin is added to the project correctly, is that still the case here or does someone need to manually perform extra actions?

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Dec 15, 2019

If this works as advertised I think its a great step forward. But not being super experienced with Android I'll have to take your word for it :)

This may solve the deploy issues for ARCore as well, might need to think of exposing a part of the camera server through GDNative first.

How is this working in conjunction with GDNatives config? Right now GDNative will do the work in making sure the plugin is added to the project correctly, is that still the case here or does someone need to manually perform extra actions?

@BastiaanOlij can you point to the GDNative logic that setups the plugin for the project so I can validate.
From my investigations, it looks like for Android plugins, the only requirement is that the GDNative shared libs be part of the apk's build path, which the new architecture still enforces so everything should work as expected.

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 2 times, most recently from 48784ef to d285659 Compare December 16, 2019 02:27
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Dec 26, 2019

@akien-mga This PR is ready on my side, so it should be ready for merging into the 4.0 branch whenever you've reviewed it.
The matching documentation update is available @ godotengine/godot-docs#2979

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 3 times, most recently from 569ee84 to 3a9465c Compare January 12, 2020 23:45
@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 3 times, most recently from 1b7d06e to 8289c50 Compare January 28, 2020 04:31
@akien-mga
Copy link
Member

akien-mga commented Feb 23, 2020

CC'ing some community members who maintain Android plugins:

If you can, please provide feedback on this proposal, which would require porting plugins to a new system again (sorry!) but should hopefully be much cleaner and correct for the Android ecosystem.
@m4gr3d also mentioned that he'd be up for helping users port their plugins.

This PR is for 4.0, but there's also a matching PR for 3.2.x: #36336.
We need to also discuss if we're OK breaking compat in 3.2.x (likely 3.2.2 if we do), or if we should keep supporting both plugin systems in 3.2.x (would be a bit messy, but better for compatibility) or keep the 3.2 system unchanged.

@DrMoriarty
Copy link
Contributor

@akien-mga It looks interesting, but I can not find example plugin implementation. Link to oculus mobile plugin seems to be broken.
Is there any plugin example?

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

@akien-mga It looks interesting, but I can not find example plugin implementation. Link to oculus mobile plugin seems to be broken.
Is there any plugin example?

@DrMoriarty The proper link is https://github.com/m4gr3d/godot_oculus_mobile/tree/2.0/plugin. I've fixed it in the description also.
Let me know if you have any questions.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

@akien-mga It looks interesting, but I can not find example plugin implementation. Link to oculus mobile plugin seems to be broken.
Is there any plugin example?

@DrMoriarty The proper link is https://github.com/m4gr3d/godot_oculus_mobile/tree/2.0/plugin. I've fixed it in the description also.
Let me know if you have any questions.

@DrMoriarty I've also migrated the GodotPaymentV3 plugin in this PR as another example.

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 2 times, most recently from 1b00b72 to cfda47e Compare February 23, 2020 15:13
@Shin-NiL
Copy link

One of the great thing about the Godot plugin (v1) was the possibility to the final user to customize their Android manifest file. Will be it possible with v2?

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

Looks good overall, some nitpicks on indentation mixup.

The main question is what kind of compatibility we want to have with the past plugin system(s). If we don't provide support for those, we should drop the v2 distinction which is just an historical detail but not relevant for plugin makers and much less plugin users (what matters is what plugin system is documented for a given Godot version).

@akien-mga For 4.0 that sounds reasonable. It's a bit tricky for 3.2.x since depending on how we got about it, there's a chance for the two systems to coexist for a short period of time. In that case, we'd need to differentiate them to both plugin makers and users.

The versioning also gives additional flexibility to engine developers for improving the plugin system in the future. If we decide to add new features, we can separate the plugin logic based on the version number, and thus ensure we maintain backward compatibility.

So at the very least, while we can remove the version from the documentation, it may be useful to keep it in the internal plugin logic.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

One of the great thing about the Godot plugin (v1) was the possibility to the final user to customize their Android manifest file. Will be it possible with v2?

@Shin-NiL Yes that's still possible and it's vastly improved as further customization can be made using the AndroidManifest syntax instead of Godot specific syntax.

@Shin-NiL
Copy link

Yes that's still possible and it's vastly improved as further customization can be made using the AndroidManifest syntax instead of Godot specific syntax.

@m4gr3d how exactly is it done, where the plugin user will edit it? I don't get it, sorry :(

@yalcin-ata
Copy link

I am fine with it if the compatibility breaks. A hard cut is (in my opinion) better than to support different systems.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

Yes that's still possible and it's vastly improved as further customization can be made using the AndroidManifest syntax instead of Godot specific syntax.

@m4gr3d how exactly is it done, where the plugin user will edit it? I don't get it, sorry :(

@Shin-NiL With the new system, each plugin has its own AndroidManifest and build.gradle files so there are a couple scenarios:

  • In the first scenario, if the plugin maker requires some additional dependencies, or additional AndroidManifest customizations, these customizations can be added directly to the AndroidManifest (or build.gradle) for the plugin by the plugin maker. At compile time, the Android build system will automatically merge the plugin's AndroidManifest with the final binary's AndroidManifest.

  • In the second scenario if the plugin requires some customizations/meta-data from the user, the user can add them to the app's AndroidManifest file. Since at compile time the manifest file are merged, the plugin will then be able to pick up the customizations added by the user.

If you have an example for the situation you're describing, I can provide an hello world reference implementation to illustrate.

@Shin-NiL
Copy link

@m4gr3d nice, I was talking about the second scenario you mentioned.
A hello world plugin will be welcome, I'm a little slow to learn new things and a example can always be helpful.

Thanks for explaning.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 23, 2020

@m4gr3d nice, I was talking about the second scenario you mentioned.
A hello world plugin will be welcome, I'm a little slow to learn new things and a example can always be helpful.

Thanks for explaning.

@Shin-NiL I've added a reference Hello World implementation: https://github.com/m4gr3d/Godot-Android-Plugins/tree/master/hello_world

Hope that helps!

@Shin-NiL
Copy link

@m4gr3d thank you very much.
So, as a plugin developer, I'll just need to distribute an .aar file right?
It's very neat. I'm fine with that.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Feb 24, 2020

@Shin-NiL yep, that's exactly right! The other neat thing is that since a plugin is now just a regular Android library, you can leverage the vast Java and Android ecosystem to build the desired plugin functionality (e.g: ML, opencv, authentication libraries)

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 3 times, most recently from f7509b5 to fecbd94 Compare March 2, 2020 19:26
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase but based on my testing of the 3.2 PR, this seems good to go.

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch 2 times, most recently from 5077a50 to 1bf51e8 Compare March 5, 2020 16:36
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Mar 5, 2020

@akien-mga the PR was rebased.

@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch from 1bf51e8 to 3a0f737 Compare March 5, 2020 17:03
@m4gr3d m4gr3d force-pushed the rearch_godot_android_plugin branch from 3a0f737 to f097def Compare March 5, 2020 18:00
@akien-mga akien-mga merged commit 93f7c63 into godotengine:master Mar 5, 2020
@akien-mga
Copy link
Member

Thanks a lot!

@m4gr3d m4gr3d deleted the rearch_godot_android_plugin branch March 5, 2020 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants