Missing assembly definitions for dissonance plugins

[sminozhenko]

Hi, thank you for the fix #161. But now we have the same kind of issue with plugins for Dissonance (in our case it's Mirror).

1. Assembly definitions files are missing (as a workaround I've added it manually)
2. NotNullAttribute class is still internal and can't be accessed outside dissonance package, but it's used in editor part of mirror plugin.

[martindevans]

The way it's set up is that Dissonance itself (Assets/Plugins/Dissonance) is in an assembly definition but the integrations (Assets/Dissonance) are not. This is because most of the network assets that we integrate with are themselves not in an assembly definition, so our integration code with them cannot be either.

Did you have a problem with the plain Dissonance & Dissonance+Mirror packages from the store, or is this only a problem if you add your own asmdef?

[sminozhenko]

Hi, Just installed Dissonance+Mirror+Dissonance plugin and it works fine. But when I add own asmdef files for Dissonance plugin (code and editor) the error about NotNullAttribute class appears

Assets\Plugins\Dissonance\Integrations\MirrorIgnorance\MirrorIgnoranceClient.cs(15,39): error CS0122: 'NotNullAttribute' is inaccessible due to its protection level

[martindevans]

Unfortunately I don't think there's anything I can do to fix this at the moment - I can't makes the NotNullAttribute (and everything else in that file) public because those annotations are fairly common; if two assets included them and both made them public you would have a build error due to the conflict (two definitions of the same thing) but the build error would not really be the fault of either asset!

Hopefully this will be a temporary situation. Once all (or most) of the third party assets that we integrate with are in asmdefs I can move all of the integration code into asmdefs too.

Fortunately you can add a fairly simple workaround for this. If you add a csharp file into the assembly containing NotNullAttribute you can put this into it:

[assembly: InternalsVisibleTo("TheNameOfYourAssembly")]

This will make the internals of that assembly visible to your assembly, which will make NotNullAttribute accessible without making it publicly accessible and causing potential conflicts).

[Alexees]

@sminozhenko Wanted to chime in as I ran into this kind of problem myself. In my case, I had knit the ASMDEFs together the wrong way. NotNullAttribute is part of Jetbrains.cs, which exists twice and should be referenced correctly if properly setup. Check and follow this exactly: https://dissonance.readthedocs.io/en/latest/Tutorials/Assembly-Definitions/index.html

[sminozhenko]

@Alexees AsmDefs are now added in the package, so you don't need to perform these steps manually. But when you add asmdefs for certain plugin, like DIssonance-Mirror - classes from Jetbrains.cs are not accessible because of internal access modifier.

@martindevans Mirror already has asmdef available and for the related plugin, we can already add AsmDefs files. Also, I found the article that JB annotations are part of Unity already (https://forum.unity.com/threads/embedded-jetbrains-annotations-in-v5-unityengine-dll.304819/) not sure if it's up to date and hasn't check this yet.

Anyway, it's not a big issue for me to change the access modifier for NotNullAttribute.

[Alexees]

@sminozhenko The error you describe comes from the DissonanceIntegrations not being present in the Assets/Dissonance folder. I assume you made the same mistake putting in under Assets/Dissonance/Integrations

[martindevans]

JB annotations are part of Unity already

Oh wow that's a great find! I just checked and if I delete Jetbrains.cs and add using JetBrains.Annotations; to every file it all works (in 2017.4). I won't release an update to every single integration just to make that one change, but as we release future updates I'll include this change and we can slowly phase out Jetbrains.cs altogether. Thanks very much for pointing this out to me.

[martindevans]

We've had asmdefs since Dissonance 6.4.3 and with Dissonance 7.0.1 we've removed the Jetbrains.cs file that we used to ship.