Skip to content

Conversation

@suriksarkisyan
Copy link
Contributor

No description provided.

Comment on lines +56 to +59




Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +44 to +47




Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change

{
if (!(Json.Deserialize(jsonStr) is Dictionary<string, object> screenResult))
{
Debug.LogError("Could not parse NoCodes screen id");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Debug.LogError("Could not parse NoCodes screen id");
Debug.LogError("Could not parse No-Code Screen id");

Comment on lines +63 to +84
case "nocodes_screen_shown":
case "screen_shown":
result = NoCodesEventType.ScreenShown;
break;
case "nocodes_finished":
case "finished":
result = NoCodesEventType.Finished;
break;
case "nocodes_action_started":
case "action_started":
result = NoCodesEventType.ActionStarted;
break;
case "nocodes_action_failed":
case "action_failed":
result = NoCodesEventType.ActionFailed;
break;
case "nocodes_action_finished":
case "action_finished":
result = NoCodesEventType.ActionFinished;
break;
case "nocodes_screen_failed_to_load":
case "screen_failed_to_load":
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are keys duplicated? let's syncronize them

/// <summary>
/// Event fired when any NoCodes event occurs.
/// </summary>
event Action<NoCodesEvent> EventReceived;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it needed, when we have a delegate?

Comment on lines +23 to +36
Task SetScreenPresentationConfig(NoCodesPresentationConfig config, [CanBeNull] string contextKey = null);

/// <summary>
/// Shows a NoCodes screen with the specified context key.
/// </summary>
/// <param name="contextKey">Context key for the screen to show.</param>
/// <returns>Task that completes when the screen is shown.</returns>
Task ShowScreen(string contextKey);

/// <summary>
/// Closes the current NoCodes screen.
/// </summary>
/// <returns>Task that completes when the screen is closed.</returns>
Task Close();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use tasks here, not void?

Comment on lines +16 to +17
private const string SdkVersion = "10.0.3";
private const string SdkSource = "unity";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like duplication of these constants, let's use the ones from QonversionInternal - either move them somewhere or somehow else

Comment on lines +59 to +68
/// <summary>
/// Simplified initialization method that only requires a project key.
/// </summary>
/// <param name="projectKey">Qonversion project key.</param>
/// <returns>Initialized instance of the NoCodes SDK.</returns>
public static INoCodes InitializeWithProjectKey(string projectKey)
{
var config = new NoCodesConfig(projectKey);
return Initialize(config);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need it

/// <summary>
/// Builder class for creating NoCodesConfig instances.
/// </summary>
public class NoCodesConfigBuilder
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is an option to provide a delegate from here?

/// Sets the delegate for handling NoCodes events.
/// </summary>
/// <param name="delegate">Delegate instance for handling NoCodes events.</param>
public static void SetDelegate(NoCodesDelegate @delegate)
Copy link
Contributor

Choose a reason for hiding this comment

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

There shouldn't be a static method for that; it's set either through initialization via config or using the initialized instance.

Comment on lines +45 to +47
unsigned long len = strlen(unityListener);
noCodesUnityListenerName = malloc(len + 1);
strcpy(noCodesUnityListenerName, unityListener);
Copy link
Contributor

Choose a reason for hiding this comment

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

so strange to see it in 2025 :D

Comment on lines +53 to +56
if (!noCodesSandwich) {
NSLog(@"Error: NoCodesSandwich not initialized. Call _initializeNoCodes first.");
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these checks here and below are extra, as we are responsible for initialization of the sandwich. If it crashes here, we will see the error during the development, but these checks make the code less readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants