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-implement GodotPayment Android plugin using the Google Play Billing library #39034

Merged
merged 1 commit into from
May 25, 2020
Merged

Re-implement GodotPayment Android plugin using the Google Play Billing library #39034

merged 1 commit into from
May 25, 2020

Conversation

timoschwarzer
Copy link
Contributor

@timoschwarzer timoschwarzer commented May 25, 2020

What does this PR do?

Re-implement the GodotPayment Android plugin to migrate away from AIDL ("Accessing Google Play Billing using AIDL is deprecated, and all integrations must use the Google Play Billing Library in the future" source) and to using signals. This also enables the usage of in-app subscriptions.

This breaks compatibility but we already broke compatibility by renaming GodotPayments to GodotPayment for 3.2.2 and the changes should be fairly trivial to implement in existing games (I'll write docs once this PR is done).

Example code using the new API

Some methods are missing from this example, I'll add them later... (purchasing items, consuming items, querying SKU details)

func _ready():
	if Engine.has_singleton("GodotPayment"):
		payment = Engine.get_singleton("GodotPayment")
		payment.connect('connected', self, '_on_connected') # No params
		payment.connect('disconnected', self, '_on_disconnected') # No params
		payment.connect('connect_error', self, '_on_connect_error') # Response ID (int), Debug message (string)
		payment.connect('purchases_updated', self, '_on_purchases_updated') # Purchases (Dictionary[])
		payment.connect('purchase_error', self, '_on_purchase_error') # Response ID (int), Debug message (string)
		payment.connect('sku_details_query_completed', self, '_on_sku_details_query_completed') # SKUs (Dictionary[])
		payment.connect('sku_details_query_error', self, '_on_sku_details_query_error') # Response ID (int), Debug message (string), Queried SKUs (string[])
		payment.connect('purchase_acknowledged', self, '_on_purchase_acknowledged') # Purchase token (string)
		payment.connect('purchase_acknowledgement_error', self, '_on_purchase_acknowledgement_error') # Response ID (int), Debug message (string), Purchase token (string)
		payment.connect('purchase_consumed', self, '_on_purchase_consumed') # Purchase token (string)
		payment.connect('purchase_consumption_error', self, '_on_purchase_consumption_error') # Response ID (int), Debug message (string), Purchase token (string)
		payment.startConnection()
	else:
		print("GodotPayment singleton is only available on Android devices.")

func _on_connected():
	var query = payment.queryPurchases('subs')
	if query.status == OK:
		for purchase in query.purchases:
			print('The player has purchased: ' + str(purchase.sku))
			if !purchase.is_acknowledged:
				print('Purchase ' + str(purchase.sku) + ' has not been acknowledged...')
				payment.acknowledgePurchase(purchase.purchase_token)

# ...all the other callbacks...

Current problems

I found no way to conditionally add a Gradle dependency to the custom Android build. com.android.billingclient:billing:2.2.1 should only be added if the module is enabled in the project export dialog because of 1. size overhead and 2. it automatically adding the BILLING permission to the app.
My current workaround is to create a file <project dir>/android/billing-client/gradle.conf with the following content to include the library in the final build:

[dependencies]
    implementation 'com.android.billingclient:billing:2.2.1'

Dependencies

If you want to test this, make sure that #39029 is merged or merge it yourself before as this PR depends on it.

To-Do

  • Find a solution to the Google Play Billing library inclusion problem
  • Fix code style to match project code style
  • Write migration guide to make it clear to users that it's now required to acknowledge purchases and subscriptions
  • Update example IAP project
  • Update docs

@timoschwarzer
Copy link
Contributor Author

Maybe there is already a solution to the library inclusion problem in the new Android plugin system that I didn't find... (CC: @m4gr3d)

@m4gr3d
Copy link
Contributor

m4gr3d commented May 25, 2020

Maybe there is already a solution to the library inclusion problem in the new Android plugin system that I didn't find... (CC: @m4gr3d)

@timoschwarzer With the new plugin system, you can define a configuration file that can specify the dependencies needed by the plugin.
Since GodotPayment is a built-in plugin, its configuration is defined here, so you just need to update it with the needed dependency.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

I'm guessing this is not opened against the master branch so that you can test and validate?

@timoschwarzer
Copy link
Contributor Author

timoschwarzer commented May 25, 2020

With the new plugin system, you can define a configuration file that can specify the dependencies needed by the plugin.

Thank you for the hint @m4gr3d, that works :)! Although I didn't find a better way to create an inline Vector<String> than this:

/*
 * Set of prebuilt plugins.
 */
static const PluginConfig GODOT_PAYMENT = {
	/*.valid_config =*/true,
	/*.name =*/"GodotPayment",
	/*.binary_type =*/"local",
	/*.binary =*/"res://android/build/libs/plugins/GodotPayment.release.aar",
	/*.local_dependencies =*/{},
	/*.remote_dependencies =*/String("com.android.billingclient:billing:2.2.1").split("|"),
	/*.custom_maven_repos =*/{}
};

There has to be a better way...

@timoschwarzer
Copy link
Contributor Author

I'm guessing this is not opened against the master branch so that you can test and validate?

This is opened against 3.2 for now because @akien-mga said that we can break compatibility now with 3.2.2 if necessary as we already kind of broke it by renaming GodotPayments to GodotPayment. I can also imagine Google rejecting apps using the old AIDL API in the near future because their warnings are quite bold. (1, 2)

When everything works in 3.2, I'll start testing with master :)

@timoschwarzer
Copy link
Contributor Author

@m4gr3d Thanks a lot for the review! All valid points and done :)

@timoschwarzer timoschwarzer requested a review from m4gr3d May 25, 2020 17:03
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

The code looks good to me!

@akien-mga
Copy link
Member

There are some code styling issues: https://travis-ci.org/github/godotengine/godot/jobs/691014967

You can set up clang-format locally to apply changes automatically: https://docs.godotengine.org/en/latest/community/contributing/code_style_guidelines.html

@timoschwarzer
Copy link
Contributor Author

@akien-mga fixed 👍

@m4gr3d are there significant differences between the Android build systems in 3.2 and master? I'd start the PR for 4.0 now :)

@m4gr3d
Copy link
Contributor

m4gr3d commented May 25, 2020

@m4gr3d are there significant differences between the Android build systems in 3.2 and master? I'd start the PR for 4.0 now :)

@timoschwarzer No the changes should be identical.
The previous v0 system is completely deprecated and removed in 4.0, but the v1 system introduced in this upcoming release and the one you're using, is made forward-compatible with 4.0.

@akien-mga
Copy link
Member

Thanks!

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

Engine.has_singleton("GodotPayment") now returns false even running on Android device.
I compiled editor and android template with this PR merged.
cc @timoschwarzer

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

I got error log from adb logcat.

05-27 13:07:34.469: I/GodotPluginRegistry(22888): Initializing Godot plugin GodotPayment
05-27 13:07:34.470: I/zygote64(22888): Rejecting re-init on previously-failed class java.lang.Class<org.godotengine.godot.plugin.payment.GodotPayment>: java.lang.NoClassDefFoundError: Failed resolution of: Lcom/android/billingclient/api/PurchasesUpdatedListener;
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.classForName(java.lang.String, boolean, java.lang.ClassLoader) (Class.java:-2)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.forName(java.lang.String, boolean, java.lang.ClassLoader) (Class.java:453)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.forName(java.lang.String) (Class.java:378)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.plugin.GodotPluginRegistry.loadPlugins(org.godotengine.godot.Godot) (GodotPluginRegistry.java:173)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.plugin.GodotPluginRegistry.<init>(org.godotengine.godot.Godot) (GodotPluginRegistry.java:71)
05-27 13:07:34.470: I/zygote64(22888):   at org.godotengine.godot.plugin.GodotPluginRegistry org.godotengine.godot.plugin.GodotPluginRegistry.initializePluginRegistry(org.godotengine.godot.Godot) (GodotPluginRegistry.java:103)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.Godot.onCreate(android.os.Bundle) (Godot.java:574)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Activity.performCreate(android.os.Bundle, android.os.PersistableBundle) (Activity.java:7009)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Activity.performCreate(android.os.Bundle) (Activity.java:7000)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Instrumentation.callActivityOnCreate(android.app.Activity, android.os.Bundle) (Instrumentation.java:1214)
05-27 13:07:34.470: I/zygote64(22888):   at android.app.Activity android.app.ActivityThread.performLaunchActivity(android.app.ActivityThread$ActivityClientRecord, android.content.Intent) (ActivityThread.java:2731)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.handleLaunchActivity(android.app.ActivityThread$ActivityClientRecord, android.content.Intent, java.lang.String) (ActivityThread.java:2856)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.-wrap11(android.app.ActivityThread, android.app.ActivityThread$ActivityClientRecord, android.content.Intent, java.lang.String) (ActivityThread.java:-1)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread$H.handleMessage(android.os.Message) (ActivityThread.java:1589)
05-27 13:07:34.470: I/zygote64(22888):   at void android.os.Handler.dispatchMessage(android.os.Message) (Handler.java:106)
05-27 13:07:34.470: I/zygote64(22888):   at void android.os.Looper.loop() (Looper.java:164)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.main(java.lang.String[]) (ActivityThread.java:6494)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Object java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) (Method.java:-2)
05-27 13:07:34.470: I/zygote64(22888):   at void com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run() (RuntimeInit.java:438)
05-27 13:07:34.470: I/zygote64(22888):   at void com.android.internal.os.ZygoteInit.main(java.lang.String[]) (ZygoteInit.java:807)
05-27 13:07:34.470: I/zygote64(22888): Caused by: java.lang.ClassNotFoundException: Didn't find class "com.android.billingclient.api.PurchasesUpdatedListener" on path: DexPathList[[zip file "/data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/base.apk"],nativeLibraryDirectories=[/data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/lib/arm64, /data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/base.apk!/lib/arm64-v8a, /system/lib64, /vendor/lib64]]
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class dalvik.system.BaseDexClassLoader.findClass(java.lang.String) (BaseDexClassLoader.java:125)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.ClassLoader.loadClass(java.lang.String, boolean) (ClassLoader.java:379)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.ClassLoader.loadClass(java.lang.String) (ClassLoader.java:312)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.classForName(java.lang.String, boolean, java.lang.ClassLoader) (Class.java:-2)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.forName(java.lang.String, boolean, java.lang.ClassLoader) (Class.java:453)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Class java.lang.Class.forName(java.lang.String) (Class.java:378)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.plugin.GodotPluginRegistry.loadPlugins(org.godotengine.godot.Godot) (GodotPluginRegistry.java:173)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.plugin.GodotPluginRegistry.<init>(org.godotengine.godot.Godot) (GodotPluginRegistry.java:71)
05-27 13:07:34.470: I/zygote64(22888):   at org.godotengine.godot.plugin.GodotPluginRegistry org.godotengine.godot.plugin.GodotPluginRegistry.initializePluginRegistry(org.godotengine.godot.Godot) (GodotPluginRegistry.java:103)
05-27 13:07:34.470: I/zygote64(22888):   at void org.godotengine.godot.Godot.onCreate(android.os.Bundle) (Godot.java:574)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Activity.performCreate(android.os.Bundle, android.os.PersistableBundle) (Activity.java:7009)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Activity.performCreate(android.os.Bundle) (Activity.java:7000)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.Instrumentation.callActivityOnCreate(android.app.Activity, android.os.Bundle) (Instrumentation.java:1214)
05-27 13:07:34.470: I/zygote64(22888):   at android.app.Activity android.app.ActivityThread.performLaunchActivity(android.app.ActivityThread$ActivityClientRecord, android.content.Intent) (ActivityThread.java:2731)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.handleLaunchActivity(android.app.ActivityThread$ActivityClientRecord, android.content.Intent, java.lang.String) (ActivityThread.java:2856)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.-wrap11(android.app.ActivityThread, android.app.ActivityThread$ActivityClientRecord, android.content.Intent, java.lang.String) (ActivityThread.java:-1)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread$H.handleMessage(android.os.Message) (ActivityThread.java:1589)
05-27 13:07:34.470: I/zygote64(22888):   at void android.os.Handler.dispatchMessage(android.os.Message) (Handler.java:106)
05-27 13:07:34.470: I/zygote64(22888):   at void android.os.Looper.loop() (Looper.java:164)
05-27 13:07:34.470: I/zygote64(22888):   at void android.app.ActivityThread.main(java.lang.String[]) (ActivityThread.java:6494)
05-27 13:07:34.470: I/zygote64(22888):   at java.lang.Object java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) (Method.java:-2)
05-27 13:07:34.470: I/zygote64(22888):   at void com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run() (RuntimeInit.java:438)
05-27 13:07:34.470: I/zygote64(22888):   at void com.android.internal.os.ZygoteInit.main(java.lang.String[]) (ZygoteInit.java:807)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): Unable to load Godot plugin GodotPayment
05-27 13:07:34.475: W/GodotPluginRegistry(22888): java.lang.ClassNotFoundException: org.godotengine.godot.plugin.payment.GodotPayment
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.Class.classForName(Native Method)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.Class.forName(Class.java:453)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.Class.forName(Class.java:378)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at org.godotengine.godot.plugin.GodotPluginRegistry.loadPlugins(GodotPluginRegistry.java:173)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at org.godotengine.godot.plugin.GodotPluginRegistry.<init>(GodotPluginRegistry.java:71)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at org.godotengine.godot.plugin.GodotPluginRegistry.initializePluginRegistry(GodotPluginRegistry.java:103)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at org.godotengine.godot.Godot.onCreate(Godot.java:574)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.Activity.performCreate(Activity.java:7009)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.Activity.performCreate(Activity.java:7000)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1214)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2731)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2856)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.ActivityThread.-wrap11(Unknown Source:0)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1589)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.os.Handler.dispatchMessage(Handler.java:106)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.os.Looper.loop(Looper.java:164)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at android.app.ActivityThread.main(ActivityThread.java:6494)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.reflect.Method.invoke(Native Method)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): Caused by: java.lang.ClassNotFoundException: org.godotengine.godot.plugin.payment.GodotPayment
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.VMClassLoader.findLoadedClass(Native Method)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.ClassLoader.findLoadedClass(ClassLoader.java:738)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.ClassLoader.loadClass(ClassLoader.java:363)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	... 20 more
05-27 13:07:34.475: W/GodotPluginRegistry(22888): Caused by: java.lang.NoClassDefFoundError: Failed resolution of: Lcom/android/billingclient/api/PurchasesUpdatedListener;
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	... 20 more
05-27 13:07:34.475: W/GodotPluginRegistry(22888): Caused by: java.lang.ClassNotFoundException: Didn't find class "com.android.billingclient.api.PurchasesUpdatedListener" on path: DexPathList[[zip file "/data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/base.apk"],nativeLibraryDirectories=[/data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/lib/arm64, /data/app/com.test-s1tfq_9jXDaeuohJv90BvQ==/base.apk!/lib/arm64-v8a, /system/lib64, /vendor/lib64]]
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:125)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
05-27 13:07:34.475: W/GodotPluginRegistry(22888): 	... 20 more

@timoschwarzer
Copy link
Contributor Author

@volzhs have you recompiled your editor? You need to do this because the dependencies for the included Android plugins changed.

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

@timoschwarzer yes. I've always compiled both editor and template.

@timoschwarzer
Copy link
Contributor Author

@volzhs just to be sure: have you re-installed your Android build template?

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

@timoschwarzer yeap. replaced new templates every time on custom directory path which is defined at export setting.

@timoschwarzer
Copy link
Contributor Author

timoschwarzer commented May 27, 2020

@volzhs I just cloned the repository into a new directory (godot3). These are the steps I did and it worked just fine.

git clone https://github.com/godotengine/godot godot3
cd godot3
git checkout 757d8b56721f856e4bd5ec7f27b8bc619ddc2bca
scons platform=x11 bits=64 tools=yes target=release_debug -j 16
scons platform=android bits=64 target=release_debug android_arch=arm64v8 -j 16
scons platform=android bits=64 target=release android_arch=arm64v8 -j 16
scons platform=android bits=64 target=release_debug android_arch=armv7 -j 16
scons platform=android bits=64 target=release android_arch=armv7 -j 16
cd platform/android/java
./gradlew generateGodotTemplates
cd ../../../

# Copy all ZIP, APK and AAR files from ./bin to ~/.local/share/godot/templates/3.2.2.beta

./bin/godot.x11.opt.tools.64

Then I created a new project, clicked Project → Install Android Build Template and added a new main scene with the following script:

extends Node

func _ready():
	print(Engine.has_singleton('GodotPayment'))

After that I added an Android export template and checked 'Use custom build' and 'Plugins / Godot Payment'.

image

Finally I checked 'Debug → Deploy with Remote Debug' clicked the Android run button in the top right corner of the editor.

Result:
image

@timoschwarzer
Copy link
Contributor Author

For some reason, your Gradle build does not include the Google Play Billing library, which leads to the error you're getting.

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

@timoschwarzer should I check Use custom build to use Godot payment plugin?

@timoschwarzer
Copy link
Contributor Author

timoschwarzer commented May 27, 2020

@volzhs You have to because Gradle won't fire up and install the dependencies otherwise. But yeah, this should be noted in the docs. I didn't have time to update the docs yet but I'll do that the next days. Thank you for reminding me to add a bold notice for that requirement!

@akien-mga
Copy link
Member

I missed that initially, do we want to force uses to use a custom build to use GodotPayment? If so, it should not be exposed as a togglable plugin when "Use Custom Build" is off.

The alternative is to add the billing library in the base APK I guess? But that would force the BILLING permission on?

@m4gr3d
Copy link
Contributor

m4gr3d commented May 27, 2020

@akien-mga @timoschwarzer Unfortunately yes. It's either making the default Android templates include the BILLING permission in addition of the billing library, or requiring the use of custom build.

It's hard trade off because Use custom build is harder than the alternative, but we could argue that is the user is setting up billing, which does require configuration, than configuring the Android build tools should be a small ask.

If we end up requiring Use Custom Build for GodotPayment, we can decouple it completely (as now the code is still included in the default template) as a standalone plugin.

Do we have a hard date for the deprecation of the previous API, as it would simplify the decision?

@akien-mga
Copy link
Member

I assume that users who use IAP will often want to use other Android plugins like ad providers or Google Play Services, so it's probably OK to make it conditional to doing a custom build.

But indeed, if this is the new requirement, then decoupling it completely from the one-click deploy APK would be good.

@timoschwarzer
Copy link
Contributor Author

timoschwarzer commented May 27, 2020

If so, it should not be exposed as a togglable plugin when "Use Custom Build" is off.

It's hard trade off because Use custom build is harder than the alternative, but we could argue that is the user is setting up billing, which does require configuration, than configuring the Android build tools should be a small ask.

I agree to both of you. My proposal would be adding a field like requires_custom_build to PluginConfig in https://github.com/godotengine/godot/blob/3.2/platform/android/plugin/godot_plugin_config.h, disabling/hiding the checkbox in the project export dialog accordingly.

If we end up requiring Use Custom Build for GodotPayment, we can decouple it completely (as now the code is still included in the default template) as a standalone plugin.

If we decouple the plugin completely, maybe it's worth setting up a first-party Android plugins repository under the godotengine namespace with the same branching scheme as this repo. (master, 3.2 etc.) This would make the above proposal obsolete, too.

@volzhs
Copy link
Contributor

volzhs commented May 27, 2020

I can connect store with GodotPayment by following all @timoschwarzer steps. thanks.
now I need to test rest of functions. like payment.querySkuDetails :)

@m4gr3d
Copy link
Contributor

m4gr3d commented May 27, 2020

I agree to both of you. My proposal would be adding a field like requires_custom_build to PluginConfig in https://github.com/godotengine/godot/blob/3.2/platform/android/plugin/godot_plugin_config.h, disabling/hiding the checkbox in the project export dialog accordingly.

@timoschwarzer All plugins asides from the built-ins (GodotPayment being the only one) require that Use custom build be enabled, so that's already implicit.

Disabling/hiding the checkbox in the project export is not possible with the current implementation. I mulled adding the implementation for it, but decided to delay it since we were already in the beta phase, and that's kind of semi-large change if done right.

What we can do instead for now, especially if we go ahead with decoupling the GodotPayment plugin, is to add a warning at export time if a plugin is enabled but Use Custom Build is disabled.
See here for an example by @akien-mga

@HEAVYPOLY
Copy link
Contributor

I've exported with use custom builds and godotpayment plugin. Engine.has_singleton("GodotPayment") returns true,
but it gets stuck if I add setpurchasecallbackid.
This was working fine with GodoyPaymentsV3

Is there a guide somewhere on how to get this working with the new system?

if Engine.has_singleton("GodotPayment"):
    payment = Engine.get_singleton("GodotPayment")
    payment.setPurchaseCallbackId(get_instance_id())

@timoschwarzer
Copy link
Contributor Author

@HEAVYPOLY not yet. I'll begin updating the example project today. Please take a look at "Example code using the new API" above or ping me on Discord/IRC if you need help :)

@timoschwarzer
Copy link
Contributor Author

@akien-mga Where would be the best place for a migration guide for existing GodotPayments users?

@akien-mga
Copy link
Member

Probably on the official docs. I think there's a page covering IAP, not sure how much details it has regarding the GodotPayments API but I guess it needs both an update + a section with a migration guide.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 18, 2020

@timoschwarzer Since 3.2.2.stable is fast approaching, is this update to the GodotPayment plugin feature complete and stable enough for release with 3.2.2.stable?

@timoschwarzer
Copy link
Contributor Author

@m4gr3d it's feature-complete and I've been using it in production for subscriptions with no problems. Example projects are updated and I opened a PR for the docs :)

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 18, 2020

@timoschwarzer cool!

I've pinged @akien-mga to discuss if we should move the plugin into its own repo prior to releasing 3.2.2.stable.
That would allow us to quickly address any issues encountered by users, as well as to quickly perform updates (e.g: Google Play Billing v3).

@timoschwarzer
Copy link
Contributor Author

@m4gr3d makes sense IMO since it requires Custom Build anyways.

@akien-mga
Copy link
Member

@m4gr3d @timoschwarzer Moving it to a separate repo makes sense, since we're breaking compatibility anyway we might as well go all the way. We should do this in coming days, I don't want to delay the 3.2.2-stable release too long. But we can take the time needed to do it right of course.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 18, 2020

@akien-mga I expect the code migration process to be quick. I can perform the migration, we just need to agree on the new repo name and structure. @timoschwarzer Your input would also be appreciated :)

@timoschwarzer Once the code is migrated, you'll need to update the documentation accordingly (both godot docs, and the new repo's README).

@akien-mga
Copy link
Member

Naming wise, I don't have a strong opinion, beyond using kebab-case, so maybe something like godotpayment-android-plugin?

Note that given the scope of the changes, if you think a different name would be better than GodotPayment, that's also an option.

@timoschwarzer
Copy link
Contributor Author

@akien-mga @m4gr3d what about godot-google-billing as repo name and renaming the singleton to GoogleBilling? In my opinion GodotPayment doesn't make clear that it is Android only at first sight. And the new GodotPayment (this PR) it's more or less a plain wrapper around the Google Billing library.

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 18, 2020

@timoschwarzer sounds fine by me. It would have to be godot-google-play-billing and GooglePlayBilling though since this is how it's referred to in the documentation.

@timoschwarzer
Copy link
Contributor Author

timoschwarzer commented Jun 18, 2020

@m4gr3d yeah, Google Play Billing of course. (it's late here 😅)

@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 18, 2020

@timoschwarzer I've created the repo and sent you an invite. Feel free to start updating the repo description and README.
For the repo files updates, use pull requests so changes can be reviewed and we can ensure we're not stepping over each other.

@akien-mga
Copy link
Member

akien-mga commented Jun 18, 2020

Sounds good to me.

@m4gr3d Feel free to start the migration then. I think you should be able to create a new repo under the @godotengine org, if not, you can make it in your namespace and we'll see how to move it when I'm back online tomorrow :)

Edit: forsooth, you're fast :D

@timoschwarzer
Copy link
Contributor Author

@m4gr3d @akien-mga Thank you! I'll work on that tomorrow. I'll create an additional PR in the main repository that removes the plugin from the tree.

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.

5 participants