Custom Read Write functions for Serialization not working over mulitple assembly definitions

[NicatorBa]

Describe the bug The custom data type with its read-write functions for serialization and the NetworkBehaviour used are in different assembly definitions. When calling the ClientRpc the Custom Read Write functions are apparently not used.

Repro project is gone but see comment below for possible fix.

=============================================

Repro project https://github.com/NicatorBa/MirrorCustomReadWrite

To Reproduce Steps to reproduce the behavior:

1. Open the attached project
2. Select 'SampleScene'
3. Click on 'Play'
4. Click on 'Host'
5. Wait 10 seconds
6. See debug output in 'Console' from Transmitter class

Expected behavior In the console, the values generated by the server should be output on client side.

Desktop:

• OS: Windows
• Build target: standalone
• Unity version: 2019.4.12f1
• Mirror tag: 26.2.2

[MrGadget1024]

Duplicate and should be already fixed in Mirror 30.2.1 that's now live on asset store. Update Mirror and restart Unity and comment here if issue persists.

[NicatorBa]

Updated via the Asset Store Mirror, but the described behavior still exists.

[MrGadget1024]

Reopening - reference #2424

[miwarnec]

Describe the bug The custom data type with its read-write functions for serialization and the NetworkBehaviour used are in different assembly definitions. When calling the ClientRpc the Custom Read Write functions are apparently not used.

Repro project https://github.com/NicatorBa/MirrorCustomReadWrite

To Reproduce Steps to reproduce the behavior:

1. Open the attached project

2. Select 'SampleScene'

3. Click on 'Play'

4. Click on 'Host'

5. Wait 10 seconds

6. See debug output in 'Console' from Transmitter class

Expected behavior In the console, the values generated by the server should be output on client side.

Desktop:

* OS: Windows

* Build target: standalone

* Unity version: 2019.4.12f1

* Mirror tag: 26.2.2

sorry for late reply, can you confirm that this issue still exists?

[NicatorBa]

Thank you for the feedback. After updating to 40.0.9 in the repro project, the behavior is still as described. Unfortunately, the issue still exists.

[jhaverkost]

Since the repro project isn't available anymore, it is hard for me to say if the following is the same situation @NicatorBa was experiencing but I have more information about why custom read/write methods are not working with multiple assemblies. Unfortunately, I do not have a repro project available.

Mirror assembly weaving appears to be done alphabetically NOT by a dependency tree order. This has big consequences for the code handling network serialization.

Suppose we have assemblies A, B, and C (named as such because the alphabetical order is important):

• Assembly A
  • no relevant dependencies
  • simply contains Plain Old C# Objects (POCO) which are not supported by Mirror's automatic serialization (for example, due to no default constructor).
• Assembly B
  • depends on Assembly C
  • contains networking code that passes the POCO back and forth across the network using Command and RPCs which requires serialization.
• Assembly C
  • depends on Assembly A
  • contains custom serialization code for the POCO in order to keep the networking separate from the data definitions (e.g. needing to add networking serialization support for a third party assembly that cannot be modified).

In the above scenario, the project will not compile due to Mirror assembly weaving failures. Assembly B will be processed before Assembly C (due to alphabetical order) so the custom serialization code will not be detected and Mirror will attempt to automatically generate a reader/writer which fails (e.g. "can't be deserialized because it has no default constructor").

[James-Frowen]

Mirror assembly weaving appears to be done alphabetically NOT by a dependency tree order. This has big consequences for the code handling network serialization.

If I remember correctly it does run in dependency order not alphabetical order. The order is not the problem

The problem is this:

• When an Assembly is compiled Weaver only looks current assembly for write/read functions, and if it does not find them it will try to create its own.
• Because of this B will create it's own even if C has valid functions.
• If B's type can't be generated by weaver it will fail to compile, even if there are valid functions for the type inside C

Workaround: Include custom write/read functions for type in Assembly that uses the type

[jhaverkost]

Thanks for the response. I have spent pretty much all afternoon working through this. I enabled the logging for the ILPostProcessorHook and it is consistently in alphabetical order for me but that just might be a coincidence.

When an Assembly is compiled Weaver only looks current assembly for write/read functions, and if it does not find them it will try to create its own.

Like you said, this appears to be the biggest issue. I tracked this down and was actually working on the workaround below and it appears to work just fine so far but I don't entirely know what the consequences are since I am not very familiar with any of this.

Original:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

Workaround:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find NetworkReader/Writer extensions in referenced assemblies
    // save a copy of the collection enumerator since it appears to be modified at some point during iteration
    IEnumerable<AssemblyNameReference> assemblyReferences = CurrentAssembly.MainModule.AssemblyReferences.ToList();
    foreach (AssemblyNameReference assemblyNameReference in assemblyReferences)
    {
        AssemblyDefinition referencedAssembly = resolver.Resolve(assemblyNameReference);
        if (referencedAssembly != null)
        {
            ProcessAssemblyClasses(CurrentAssembly, referencedAssembly, writers, readers, ref WeavingFailed);
        }
    }

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

That being said, your proposed workaround is a lot safer.

[miwarnec]

Thanks for the response. I have spent pretty much all afternoon working through this. I enabled the logging for the ILPostProcessorHook and it is consistently in alphabetical order for me but that just might be a coincidence.

When an Assembly is compiled Weaver only looks current assembly for write/read functions, and if it does not find them it will try to create its own.

Like you said, this appears to be the biggest issue. I tracked this down and was actually working on the workaround below and it appears to work just fine so far but I don't entirely know what the consequences are since I am not very familiar with any of this.

Original:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

Workaround:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find NetworkReader/Writer extensions in referenced assemblies
    // save a copy of the collection enumerator since it appears to be modified at some point during iteration
    IEnumerable<AssemblyNameReference> assemblyReferences = CurrentAssembly.MainModule.AssemblyReferences.ToList();
    foreach (AssemblyNameReference assemblyNameReference in assemblyReferences)
    {
        AssemblyDefinition referencedAssembly = resolver.Resolve(assemblyNameReference);
        if (referencedAssembly != null)
        {
            ProcessAssemblyClasses(CurrentAssembly, referencedAssembly, writers, readers, ref WeavingFailed);
        }
    }

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

That being said, your proposed workaround is a lot safer.

that looks pretty good. did it work properly in your tests?

[jhaverkost]

I have been using that workaround for the past year without any issue. I am still using an older version of Mirror though.

[miwarnec]

Thanks for the response. I have spent pretty much all afternoon working through this. I enabled the logging for the ILPostProcessorHook and it is consistently in alphabetical order for me but that just might be a coincidence.

When an Assembly is compiled Weaver only looks current assembly for write/read functions, and if it does not find them it will try to create its own.

Like you said, this appears to be the biggest issue. I tracked this down and was actually working on the workaround below and it appears to work just fine so far but I don't entirely know what the consequences are since I am not very familiar with any of this.

Original:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

Workaround:

public static bool Process(AssemblyDefinition CurrentAssembly, IAssemblyResolver resolver, Logger Log, Writers writers, Readers readers, ref bool WeavingFailed)
{
    // find NetworkReader/Writer extensions from Mirror.dll first.
    // and NetworkMessage custom writer/reader extensions.
    // NOTE: do not include this result in our 'modified' return value,
    //       otherwise Unity crashes when running tests
    ProcessMirrorAssemblyClasses(CurrentAssembly, resolver, Log, writers, readers, ref WeavingFailed);

    // find NetworkReader/Writer extensions in referenced assemblies
    // save a copy of the collection enumerator since it appears to be modified at some point during iteration
    IEnumerable<AssemblyNameReference> assemblyReferences = CurrentAssembly.MainModule.AssemblyReferences.ToList();
    foreach (AssemblyNameReference assemblyNameReference in assemblyReferences)
    {
        AssemblyDefinition referencedAssembly = resolver.Resolve(assemblyNameReference);
        if (referencedAssembly != null)
        {
            ProcessAssemblyClasses(CurrentAssembly, referencedAssembly, writers, readers, ref WeavingFailed);
        }
    }

    // find readers/writers in the assembly we are in right now.
    return ProcessAssemblyClasses(CurrentAssembly, CurrentAssembly, writers, readers, ref WeavingFailed);
}

That being said, your proposed workaround is a lot safer.

Hey @jhaverkost if you are still around, is this still working well for you? Then we would just merge the Workaround part.

[miwarnec]

yeah actually this looks good, merging