godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.02k stars 21.09k forks source link

iOS SKPaymentTransactionStateFailed not generating a purchase failed event #43718

Open tommiemartin opened 3 years ago

tommiemartin commented 3 years ago

Godot version: 3.2.3

OS/device including version: iOS 12.4, Xcode 11.3

Issue description: When a transaction fails because of an itunes login error xcode debug is showing status transaction failed! Per in_app_store.mm I would expect an event.error to show in console but this is never happening

    while payment.get_pending_event_count() > 0:
        var event = payment.pop_pending_event()
#       print("EVENT KEYS: ",event.keys())

        if event.type == "purchase":
            if event.result == "ok":
                print("PURCHASE EVENT, OK: ", event.product_id)
                ios_purchases_updated( str(event.product_id)) 
                payment.finish_transaction(event.product_id)
                Events.emit_signal("working")
            else:
                print("PURCHASE EVENT, ERROR: ",event.error)
akien-mga commented 3 years ago

CC @naithar @bruvzg

naithar commented 3 years ago

From what I can tell this part is responsible for reporting transaction errors: https://github.com/godotengine/godot/blob/265caa687b6634487f5f2dcf20247534fd2a408e/platform/iphone/in_app_store.mm#L235-L243 But seems like it hasn't changed in a long time. Maybe event handling is not working? I'm not sure.

@tommiemartin can you reproduce this issue with latest Godot beta? What does Godot log into console if you uncomment print("EVENT KEYS: ",event.keys()) or can you print an event itself?

@akien-mga is there any reason to use events in iOS modules? Maybe after we merge iOS Plugin support in 3.2 we can change them to use signals instead?

akien-mga commented 3 years ago

is there any reason to use events in iOS modules? Maybe after we merge iOS Plugin support in 3.2 we can change them to use signals instead?

No idea, these iOS modules were likely written by @punto- a decade ago and haven't been modified much since, as we haven't really had a pro-active development of the iOS port until recently, beyond fixing immediate issues. (As you saw yourself while modernizing the codebase.) So if signals are more convenient/safer than events, I don't see a recent not to use them.

tommiemartin commented 3 years ago

The events print as expected however there are cases where a user cancels the transaction that no event is triggered.

In the best scenario a cancelled 'buy/password' window will show EVENT : {error:Cannot connect to iTunes Store, which can be used to unlock the store and let the user proceed

However it can also just show this with no event. status transaction failed!

This leaves my store in a locked state. Really not sure how to proceed with this. I had the launch planned soon but currently my IAP is failing Apple review.

I could just show the 'working' sprite on a short timer and leave the buttons unlocked but it feels like a bad practice. Anyone have any ideas how to work around this?

naithar commented 3 years ago

Well it seems like _post_event doesn't work sometimes for some reason. I could try changing event system to signals and add additional logging to check if it would work. Would you be able to build Godot by yourself and check it it works, if I do this change?

For a time being using a timeouts for IAP might work as a workaround.

tommiemartin commented 3 years ago

yes sure, I've built Godot with scons in the past. Would this kind of change require new export template builds as well? Seems like it shouldn't but I know Godot can be picky with the versioning numbers of these. These kind of things I can find my way through though.

Calinou commented 3 years ago

@tommiemartin Here, you only need to recompile the export templates to test this. The editor doesn't run on iOS anyway.

naithar commented 3 years ago

@tommiemartin https://github.com/naithar/godot/commit/5a6fefdb15034b998f7f40cf30ca25f8757e327a this should do it. I've removed get_pending_event_count and pop_pending_event, so you should use transaction_finished and transaction_error. If it works I'll change the names to more appropriate ones.

Just compile an export template and change .a file in Xcode project folder to compiled one. I've added additional logging, so maybe we could find the problem this way if signals wouldn't fix the issue you're experiencing.

I haven't fully tested it, so it could crash, but hopefully I've made all required checks to avoid it =/

tommiemartin commented 3 years ago

@naithar I'm seeing your NSLogs but not sure if I'm setting the signals up correctly. They are not triggering the callbacks.

    if Engine.has_singleton("InAppStore"):
        payment = Engine.get_singleton("InAppStore")

        payment.connect("transaction_finished",self,"_on_IOS_transaction_finished")
        payment.connect("transaction_error",self,"_on_IOS_transaction_error")

        payment.set_auto_finish_transaction(true)
        payment.request_product_info( { "product_ids": sku_list } )
        yield(get_tree(),"idle_frame")
        payment.restore_purchases()
func _on_IOS_transaction_finished(event_data):
    print("PURCHASE EVENT, OK: ", event_data.product_id)
    ios_purchases_updated( str(event_data.product_id)) 
    payment.finish_transaction(event_data.product_id)

func _on_IOS_transaction_error(event_data):
    print("PURCHASE EVENT, ERROR: ",event_data.error)

Am I doing something wrong here?

Also I use I was using the events event.type == "restore": with event.product_id to set my local ownership flag and event.type == "product_info": and using the returned event dictionary to populate local data like so..

func _on_sku_details_query_completed(sku_details):
               for p in ProductData.product_data:
            for i in sku_details['ids'].size() :
                if  ProductData.product_data[p]["sku"] == sku_details['ids'][i]:    
                    ProductData.product_data[p]["description"] = sku_details['descriptions'][i]
                    ProductData.product_data[p]["price"] = sku_details['localized_prices'][i]

Would it be possible to add these as signals to retain this kind of functionality?

naithar commented 3 years ago

They are not triggering the callbacks.

Does Godot log something like sending signal? From the looks of it everything should work fine.

Would it be possible to add these as signals to retain this kind of functionality?

Right now transaction_finished return every successful event even restore, so it should not be too hard. As I said, it's more of a test right now so the names might not be really describing. It will change the names before it's ready for release.

naithar commented 3 years ago

I've run tests myself with this source:

var payment = null

func _ready():
    print("start")
    if Engine.has_singleton("InAppStore"):
        print("connecting payment")
        payment = Engine.get_singleton("InAppStore")

        payment.connect("transaction_finished", self, "_on_payment_data")
        payment.connect("transaction_error", self, "_on_payment_error_data")

        payment.request_product_info( { "product_ids": ["aa", "bb"] })
        payment.restore_purchases()

func _on_payment_data(data):
    print("success: ", data)

    print("using print from issue")
    print("PURCHASE EVENT, OK: ", data.product_id)
    #ios_purchases_updated( str(data.product_id)) 
    payment.finish_transaction(data.product_id)
    print("finished using print from issue")

func _on_payment_error_data(data):
    print("error: ", data)

Got this logs from debug export template build:

2020-11-22 01:29:12.445536+0300 iOS_Touch[16937:11667802] start
start
2020-11-22 01:29:12.445599+0300 iOS_Touch[16937:11667802] connecting payment
connecting payment
************ request product info! 2
******** adding aa to product list
******** adding bb to product list
restoring purchases!
sending signal. is_error: 0
2020-11-22 01:29:12.466773+0300 iOS_Touch[16937:11668023] success: {currency_codes:[], descriptions:[], ids:[], invalid_ids:[aa, bb], localized_prices:[], prices:[], result:ok, titles:[], type:product_info}
success: {currency_codes:[], descriptions:[], ids:[], invalid_ids:[aa, bb], localized_prices:[], prices:[], result:ok, titles:[], type:product_info}
2020-11-22 01:29:12.466799+0300 iOS_Touch[16937:11668023] using print from issue
using print from issue
2020-11-22 01:29:12.466843+0300 iOS_Touch[16937:11668023] **SCRIPT ERROR**: Invalid get index 'product_id' (on base: 'Dictionary').
2020-11-22 01:29:12.466893+0300 iOS_Touch[16937:11668023]    At: res://RichTextLabel.gdc:33:_on_payment_data() - Invalid get index 'product_id' (on base: 'Dictionary').
**SCRIPT ERROR**: Invalid get index 'product_id' (on base: 'Dictionary').
   At: res://RichTextLabel.gdc:33:_on_payment_data() - Invalid get index 'product_id' (on base: 'Dictionary').
2020-11-22 01:29:22.559440+0300 iOS_Touch[16937:11667802] calling signal
sending signal. is_error: 1
2020-11-22 01:29:22.559917+0300 iOS_Touch[16937:11667802] error: {result:error, type:product_info}
error: {result:error, type:product_info}

So I guess the problem is that you try to get product_id from dictionary when it might no be having one inside. Right now transaction_finish is more of a success_callback and transaction_error is error_callback. I should have probably made names clearer, but the main goal right now is to get it working.

Could it be that the original issue is from you trying to get the field that's not really there? Are you using release export template? It might not show logs like this At: res://RichTextLabel.gdc:33:_on_payment_data() - Invalid get index 'product_id' (on base: 'Dictionary').

tommiemartin commented 3 years ago

No, that's not the case. When using event = payment.pop_pending_event()

if event.type == "restore":
            if event.result == "ok":
                ios_onRestorePuchases( str(event.product_id))

Was always returning the correct non-consumables which I would then pass to my own functions. This actually seemed to be the most stable part of it. Since it's not returning in the same event format you might have to look against the ids []

The event.keys() were [type, result, product_id, transaction_id, receipt]

naithar commented 3 years ago

Then it might be some other problem with event system.

Have you tried changing the way you work with signals? Basically you should have the same handler for both signals as for event right now, but without popping and stuff.

You should also try using debug export template.

naithar commented 3 years ago

The event.keys() were [type, result, product_id, transaction_id, receipt]

This was the keys for status transaction failed!? If it is, then print("PURCHASE EVENT, ERROR: ",event.error) wouldn't print anything as there is no error description and would stop executing the method due to the Invalid get index error.

naithar commented 3 years ago

https://github.com/naithar/godot/commit/4bd363315feb59111314896fffabbfcf2ef20c30 I've added an error log, just to check if there are some specific error without description from IAP. This should also display Error Domain, maybe this is some specific error that doesn't have data we need.

Try changing signal implementation first to work the same way as they does for events, but test it with debug build. Also try removing print("PURCHASE EVENT, ERROR: ",event.error) or add a condition if dictionary has error field first and print something if it doesn't.

tommiemartin commented 3 years ago

No the above was for event.type == "restore" which is what I thought you were referencing. I would have to validate if all the event types return the same keys.

For event.type == "purchase" I just wanted it to unlock the store if the event result was other than 'ok' .

Anyway I think signals are the way to go here let me see if I can get the new ones to connect.

naithar commented 3 years ago

@tommiemartin without any feedback I can't proceed with the signals implementation.

I haven't found any issue with current callback events implementation. And since you haven't provided full debug logs I think the issue is probably related to your usage of nonexistent keys in event dictionary. As I said earlier you should use debug template to test your application for such cases.

bitwes commented 3 years ago

I've been delving into the IAP process and code. I just opened #44969 which might address some of the issues seen here.

Here's a simplified version of my approach. Whenever I want to take an action that should result in an event in the queue I will

It's a long timeout, but it shouldn't be encountered very often.