[Bug] Annoying warning when converting from a DocumentSnapshot to a class.

[cgascons]

[REQUIRED] Please fill in the following fields:

• Unity editor version: 2020.3.14f1 LTS
• Firebase Unity SDK version: 8.1.0
• Source you installed the SDK: Unity Package Manager
• Problematic Firebase Component: Database
• Other Firebase Components in use: Auth, Database, Functions
• Additional SDKs you are using: None
• Platform you are using the Unity editor on: Windows
• Platform you are targeting: iOS, Android, and/or desktop
• Scripting Runtime: Mono

[REQUIRED] Please describe the issue here:

We are performing a very simple query which gets from the Firestore database a document, which comes in a DocumentSnapshot format and we are trying to convert it to a ScriptableObject, everything works just right, but there's this annoying warning in the Unity console which we can't get rid of and we were wondering if there's anything we might be doing wrong.

Steps to reproduce:

Have you been able to reproduce this issue with just the Firebase Unity quickstarts (this GitHub project)? No What's the issue repro rate? 100%

What happened?

Log trace: BidInstanceData must be instantiated using the ScriptableObject.CreateInstance method instead of new BidInstanceData.

How can we make the problem occur? Yes, just retrieve a document from the database and try to convert it to a ScriptableObject.

Relevant Code:

    IEnumerator ManageUserCreation()
    {
        //Get the user from the DB
        Task<DocumentSnapshot> userInDB = GetUserFromDB();

        //Wait till completed
        yield return new WaitUntil(() => userInDB.IsCompleted);

        if (userInDB.Exception != null)
            yield break;

        //Does the user exist?
        bool exists = userInDB.Result.Exists;
        if(!exists)
        {
            Debug.Log("User does not exist, creating...");

            //Create the user
            CreateUser();
        }
        else
        {
            Debug.Log("User exists, information retrieved.");

            //Assign info to player
            m_Player = userInDB.Result.ConvertTo<PlayerData>(); // <<<<===== This line generates the warning.
        }
    }

[dconeybe]

@cgascons Can you clarify if you're using Realtime Database or Firestore? You've specified "database" but the code looks like Firestore. Could you update your code sample to include any relevant "using" declarations? That would also help disambiguate. Oh, and could you include the definition of the PlayerData class and GetUserFromDB() method?

[cgascons]

@dconeybe Sorry, you are right, we are using Firestore db. I will prepare a sample and get it ready ASAP. Will post it here.

[cgascons]

Ok, we have been able to create a standalone project and have uploaded it to this wetransfer link. Steps: 1 - You will need to just create a test project in firebase with authentication by email and a firestore database. 2- I have deleted from the sample project the configuration files (google-services.json) for security reasons, remember to add them into /Assets folder. 3 - Run the project, it will automatically create a user in Firebase Auth and after that it will check if there's a user inside Firestore's "users" custom collection. If there's not such user it will create it, 4 - Click on the "Retrieve user from Firestore" button shown in the canvas which is the one that will cause the warning we mentioned. Thanks

[dconeybe]

@cgascons Thank you for providing the reproduction app! I apologize for the delay but I haven't had a chance to try it out yet. I'm hoping to test it out early next week.

[dconeybe]

Ok I was easily able to reproduce this warning based on your sample project. Thank you for providing that! Here is the code that I used:

using Firebase;
using Firebase.Extensions;
using Firebase.Firestore;
using System.Collections;
using UnityEngine;

[FirestoreData]
public class PlayerData : ScriptableObject {
  [FirestoreProperty]
  public string m_Id { get; set; }

  [FirestoreProperty]
  public string m_Email { get; set; }
}

public class UnityBug1144 : MonoBehaviour {
  IEnumerator Start() {
    {
      var task = FirebaseApp.CheckAndFixDependenciesAsync();
      yield return new WaitUntil(() => task.IsCompleted);
      if (task.Exception != null) {
        Debug.LogError("CheckAndFixDependenciesAsync() failed: " + task.Exception);
      }
    }

    FirebaseFirestore firestore = FirebaseFirestore.DefaultInstance;
    CollectionReference collection = firestore.Collection("UnityBug1144");
    DocumentReference doc = collection.Document("player");

    DocumentSnapshot snapshot = null;
    {
      var task = doc.GetSnapshotAsync();
      yield return new WaitUntil(() => task.IsCompleted);
      if (task.Exception != null) {
        Debug.LogError("GetSnapshotAsync() failed: " + task.Exception);
      }
      snapshot = task.Result;
    }

    if (! snapshot.Exists) {
      var newPlayer = ScriptableObject.CreateInstance<PlayerData>();
      newPlayer.m_Id = "PlayerId";
      newPlayer.m_Email = "[email protected]";

      var task = doc.SetAsync(newPlayer);
      yield return new WaitUntil(() => task.IsCompleted);
      if (task.Exception != null) {
        Debug.LogError("SetAsync() failed: " + task.Exception);
      }
    } else {
      PlayerData existingPlayer = snapshot.ConvertTo<PlayerData>();
      Debug.Log("m_Id: " + existingPlayer.m_Id);
      Debug.Log("m_Email: " + existingPlayer.m_Email);
    }
  }

}

And here is the warning that resulted:

PlayerData must be instantiated using the ScriptableObject.CreateInstance method instead of new PlayerData.
UnityEngine.ScriptableObject:.ctor ()
PlayerData:.ctor ()
System.Reflection.ConstructorInfo:Invoke (object[])
Firebase.Firestore.Converters.AttributedTypeConverter/<CreateObjectCreator>c__AnonStorey2:<>m__0 ()
Firebase.Firestore.Converters.AttributedTypeConverter:DeserializeMap (Firebase.Firestore.DeserializationContext,Firebase.Firestore.FieldValueProxy)
Firebase.Firestore.Converters.ConverterBase:DeserializeValue (Firebase.Firestore.DeserializationContext,Firebase.Firestore.FieldValueProxy)
Firebase.Firestore.ValueDeserializer:Deserialize (Firebase.Firestore.DeserializationContext,Firebase.Firestore.FieldValueProxy,System.Type)
Firebase.Firestore.DocumentSnapshot:ConvertTo<PlayerData> (Firebase.Firestore.ServerTimestampBehavior)
UnityBug1144/<Start>d__0:MoveNext () (at Assets/UnityBug1144.cs:51)
UnityEngine.SetupCoroutine:InvokeMoveNext (System.Collections.IEnumerator,intptr)

[dconeybe]

So the problem is that DocumentSnapshot:ConvertTo<PlayerData>() creates a new instance of PlayerData by using its zero-arg constructor:

https://github.com/firebase/firebase-unity-sdk/blob/cb1e3bc98646aa763ca08f6507ed229ae117a6c4/firestore/src/Converters/AttributedTypeConverter.cs#L83-L85

In this case, this is not the correct way to create a new instance.

Off the top of my head, I can think of two solutions to this problem:

1. Add an overload of ConvertTo<PlayerData>() that takes a PlayerData argument into which the loaded data will be stored (rather than creating a new instance). You could then pass in an existing instance of PlayerData that was created correctly.
2. Add an overload of ConvertTo<PlayerData>() that takes a Func<PlayerData> argument that will be invoked to create a new instance of PlayerData instead of using the zero-arg constructor. You could then pass in something like () => ScriptableObject.CreateInstance<PlayerData>() to create the new instance correctly.

What are your thoughts on these approaches? I'll also discuss with my team for other ideas.

[cgascons]

Hi @dconeybe, thanks for your reply, I will test the two workarounds you provided and will let you know the results

[dconeybe]

@cgascons Sorry if I was unclear, those are not "workarounds". They are proposed changes to the Firestore Unity SDK to accommodate your use case, and I was interested in your thoughts on them. The only "fix" or "workaround" for your issue today would be to remove the ScriptableObject superclass from PlayerData.

[cgascons]

Hi @dconeybe, sure now that I read your post in detail I see that you were suggesting changes to the SDK, sorry about that.

As far as I understand, both suggested approaches would require manual work, right? At the top of my head I believe we would have to do the following steps for either solution:

1. create a PlayerData object
2. fill one by one all its attributes by reading them from the DocumentSnapshot received and
3. passing that recently created PlayerData object to the ConvertTo function

If I understood this right that would mean that we would have to manually loop through all the DocumentSnapshot attributes which is what we were trying to avoid in the first place so it wouldn't be an ideal solution (at least to us). Forgive me if I misunderstood it.

[cgascons]

Hey @dconeybe any news regarding this?

[dconeybe]

Hi @cgascons. We discussed this as a team and our latest idea is to add support for a new annotation to support the desired behavior. At the moment we've named it FirestoreDataInstanceCreator. Here is an example:

[FirestoreData]
public class PlayerData : ScriptableObject {
  [FirestoreProperty]
  public string m_Id { get; set; }

  ... 
 
  // This is an example of the proposed annotation
  [FirestoreDataInstanceCreator]
  public static PlayerData NewInstance() {
    return ScriptableObject.CreateInstance<PlayerData>();
  }
}

If present, then ConvertTo<PlayerData>() would use the annotated method to create a new instance instead of its zero-argument constructor.

Unfortunately, I cannot provide an ETA for this feature to land, but I would hope for Jan-Feb 2022 at the earliest based on our competing priorities and planned release schedule.

I'm sorry that I don't better news for you! In the meantime this is something that you will need to work around.

[cgascons]

I see, thank you for the info anyways, will wait till the feature is released. Do you mind notifying here whenever you guys release it if it's not too much asking? Thanks again

[dconeybe]

I will update this GitHub Issue once the feature is released.

[TaejunPark]

Hello, Thank you for your time.

I am trying to use derived classes for Firestore Unity and got stuck. my code is like this:

[FirestoreData]
public class SchdeuleRecord
{
    [FirestoreProperty] public List<RoundRecord> RoundRecords { get; set; }
}

[FirestoreData]
public abstract class RoundRecord {}

[FirestoreData]
public class RoundRecordA : RoundRecord {}

[FirestoreData]
public class RoundRecordB : RoundRecord {}

When I: document.ConvertTo< SchdeuleRecord >() I seems not working. not giving any message at all.

is there any way I can convert RoundRecord to RoundRecordA & RoundRecordB , of List<RoundRecord> ??

if there were any overloading method on creation, it would be great. I am asking on this thread, as it looks most similar one. Thank you!

[dconeybe]

@TaejunPark. Would you mind opening a new issue with your question? I want to avoid re-purposing this thread.