Problem: Function RestoreClient is not aysnc (fix #244, #255)

[damoncro]

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

• Have you read the CONTRIBUTING.md?
• Does your PR follow the C4 patch requirements?
• Have you rebased your work on top of the latest main?
• Have you checked your code compiles in Unreal Engine 4 and 5?
• Have you checked your basic code formatting and style is fine? (using this Linter plugin)
• If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
• If your code changes public APIs, have you incremented the plugin version number and documented your changes in the CHANGELOG.md?
• If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

[tomtau]

why does RestoreClient need to be async? To my understanding, it'll just load the session file (which ok, it could be async, but in general, sync should be fast?) and open a websocket connection (but that happens in a background)... is it just for the case to have a notification back to the game if the connection failed to be established?

[damoncro]

why does RestoreClient need to be async? To my understanding, it'll just load the session file (which ok, it could be async, but in general, sync should be fast?) and open a websocket connection (but that happens in a background)... is it just for the case to have a notification back to the game if the connection failed to be established?

Because it has noticeable blocking experience when I call it in ue 5.1, it has to be async. But previously when I tested on ue 4.27, not much blocking experience.

[damoncro]

why does RestoreClient need to be async? To my understanding, it'll just load the session file (which ok, it could be async, but in general, sync should be fast?) and open a websocket connection (but that happens in a background)... is it just for the case to have a notification back to the game if the connection failed to be established?

Two events for notifications are another purpose: One is FOnNewSessionReady, the other is FOnRestoreSessionReady. Those are very important to be used to hide the QR code and return session info to users (either reject or accept). This should fix #244 as well.

If connection failed, no events but errors.

[tomtau]

ok, I haven't tested it, but if laggy/blocking in 5.1, ok to make it async

[tomtau]

should those be moved to be private fields instead of public fields? and perhaps grouped in a class/struct with a constructor that will initialise them, so that within the actor, it's guaranteed they are initialised?

[damoncro]

oh yes, should be in private.

[damoncro]

Checked, already in private section:

play-unreal-plugin/Source/CronosPlayUnreal/Public/PlayCppSdkActor.h

Lines 208 to 242 in 8f26e0a

	private:
	static ::com::crypto::game_sdk::WalletconnectClient *_coreClient;
	static const APlayCppSdkActor *_sdk;
	// Internal session result, it will be set after successfully calling
	// `EnsureSession`
	FWalletConnectEnsureSessionResult _session_result;
	/**
	* RestoreClient delegate, called after calling
	* `RestoreClient`
	*/
	FRestoreClientDelegate OnRestoreClientDelegate;
	/**
	* InitializeWalletConnect delegate, called after calling
	* `InitializeWalletConnect`
	*/
	FInitializeWalletConnectDelegate OnInitializeWalletConnectDelegate;
	/**
	* EnsureSession delegate, called after calling `EnsureSession`
	*/
	FEnsureSessionDelegate OnEnsureSessionDelegate;
	/**
	* Used for `ConnectWalletConnect`
	*/
	FString _description;
	FString _url;
	TArray<FString> _icon_urls;
	FString _name;
	int64 _chain_id;
	EConnectionType _connection_type = EConnectionType::URI_STRING;

[damoncro]

These values are only used for function ConnectWalletConnect for users to configure when they call it:
, seems grouping them is inconvenient for users to set them (they need to make a struct first).

[damoncro]

They would be initialised when users call ConnectWalletConnect, empty by default except Connection Type.

[damoncro]

That's messy, so far I haven't found a better way to pass those values from ConnectWalletConnect to callback functions or functions called in callback functions, except private values.

[leejw51crypto]

lgtm