search

asadm / playroom-unity

9 stars 1 forks source link

Fix(SDK): Use Unity's native Color type, add init checks #16

Closed momintlh closed 10 months ago

momintlh commented 10 months ago
SaadBazaz commented 10 months ago

What about when a function is called before InsertCoin in non-mock mode? https://github.com/asadm/playroom-unity/blob/57f11c92b02558349cfe775f11954245a593c54d/Assets/PlayroomKit/PlayroomKit.cs#L97

Maybe isInitialized should be for both Mock and Non-Mock modes?

function SomeAction () {

    if (!isInitialized) Debug.Error("PlayroomKit must be initialized first. Call insertCoin.")

    if (!isRunningInBrowser()) {
        // do some action
    }
    else {
        // do some action but in mock mode
    }

}
momintlh commented 10 months ago

What about when a function is called before InsertCoin in non-mock mode?

https://github.com/asadm/playroom-unity/blob/57f11c92b02558349cfe775f11954245a593c54d/Assets/PlayroomKit/PlayroomKit.cs#L97

Maybe isInitialized should be for both Mock and Non-Mock modes?

function SomeAction () {

    if (!isInitialized) Debug.Error("PlayroomKit must be initialized first. Call insertCoin.")

    if (!isRunningInBrowser()) {
        // do some action
    }
    else {
        // do some action but in mock mode
    }

}

We have a check for that in JSLIB side already in each function.

if (!window.Playroom) {
      console.error(
        "Playroom library is not loaded. Please make sure to call InsertCoin first."
      );
      return;

Doing in Unity will lead to the same console.log.

SaadBazaz commented 10 months ago

We have a check for that in JSLIB side already in each function.

if (!window.Playroom) {
      console.error(
        "Playroom library is not loaded. Please make sure to call InsertCoin first."
      );
      return;

Doing in Unity will lead to the same console.log.

Okay that's good then. But do you think some of our functions may bypass that? Specially the ones which don't even reach JSLIB? https://github.com/asadm/playroom-unity/blob/c0c11be750c9ef1d988cf4b1d7f5fae765354825/Assets/PlayroomKit/PlayroomKit.cs#L104

SaadBazaz commented 10 months ago

Looks better to me. Tested as well. Merging.