godotengine/godot

C# Collectible .net assemblies may be breaking certain packages that rely on Castle proxies (such as Moq)

jolexxa opened this issue · 10 comments

Godot version

4.0.beta1.mono.official

System information

macOS 12

Issue description

TL;DR: Collectible C# assemblies are possibly breaking some very popular 3rd party nuget packages.

This one's a bit complicated, so please be patient with me.

I use Moq in conjunction with my own test runner go_dot_test for writing C# unit tests for 5 C# Godot packages I maintain. There are probably several hundred unit tests written using Moq across all of the packages. In Godot 3.x, everything runs fine targeting netstandard2.1.

Background: I am working on porting go_dot_test itself to Godot 4. It uses itself and Moq to test itself. So far, I've succeeded in getting GoDotTest's code ported over to run in Godot4, but the tests do not pass because Moq breaks with an obscure error in the .net6 environment. All of my stuff seems to work, but Moq does not.

What's breaking: Creating a new mock and grabbing the object breaks Moq:

var myMock = new Mock<IMyInterface>()
var mockObj = myMock.Object; // Throws System.NotSupportedException — A non-collectible assembly may not reference a collectible assembly.

I've searched around the last day or so for "A non-collectible assembly may not reference a collectible assembly" error and the only things I've found that seem relevant are these two links, which I don't fully understand.

From what I understand from the Stack Overflow post, collectible assemblies may be breaking projects which create a certain type of proxy using the Castle system. Not entirely sure as I'm way out of my depth.

If I'm reading the Godot source code correctly, Godot may actually be forcing C# assemblies to load as collectible, which may be causing the error:

public PluginLoadContext(string pluginPath, ICollection<string> sharedAssemblies,
AssemblyLoadContext mainLoadContext)
: base(isCollectible: true)
{

Here's a screenshot of the error, if it's helpful.

Screen Shot 2022-09-18 at 12 17 27 PM

Steps to reproduce

  • Create a new Godot 4 project.
  • Add a C# script.
  • Include Moq:
<PackageReference Include="Moq" Version="4.18.2" />
  • Use moq to create a mock object and reference mock.Object.

Moq will throw a NotSupportedException about collectible assemblies.

Minimal reproduction project

For a minimum reproduction sample, see the demo CollectibleAssemblies I created to demonstrate this issue.

You can debug the project using the launch configuration in VSCode.

You will need a GODOT4 environment variable pointing to the Godot4 executable on your machine for the VSCode launch.json debugger configuration to work. Or, you can edit the launch configuration in .vscode/launch.json to point directly to Godot 4 on your machine by replacing ${env:GODOT4}.

I have the same problem, but using it with Harmony. It can't work with Collecible Assemblies as well.
I need it to patch _Ready method and inject dependencies there.

UPD: I built mono with isCollectible: false and it resolved my problem.

I have the same problem, but using it with Harmony. It can't work with Collectible Assemblies as well. I need it to patch _Ready method and inject dependencies there.

UPD: I built mono with isCollectible: false and it resolved my problem.

@Maclay74 What do you mean by patching _Ready and injecting dependencies? Is that a possible work-around?

I'm not in a position to build the engine itself, so I hope this gets fixed in the next beta (or maybe I can work around it).

Is that a possible work-around?

I tried to implement DI in my application, so I needed to inject dependencies into nodes.
We don't have access to ctor, the closest we have is _Ready method. So, essentially, I hook it on IL level, call my inject() method and give its control back. So I used something called Harmony, which requires assembly to not be collectible.

Building engine is not necessary, you would need to build only c# bindings.

Anyway, let's hope that this flag isn't something important and we can have it switched back eventually. As for now, the only workaround I see is building bindings.

Sorry that it won't work in your case.

The assemblies must be collectible in order for the editor to be able to reload them after building. This is the only way to reload assemblies in .NET 6, so disabling it is not an option.

Do you need to run this in the editor? If not, then we could set isCollectible: false when not in the editor.

The assemblies must be collectible in order for the editor to be able to reload them after building. This is the only way to reload assemblies in .NET 6, so disabling it is not an option.

Hi @neikeq, thanks for taking a look! You obviously know much more about this than me.

The tests I am writing for Godot projects that use Moq do not need to be run in the editor. Essentially, I am just writing unit tests for Godot game code and running the tests inside a Godot game.

I cannot imagine a scenario where I would need to run unit tests inside the editor as a tool script — if you could change it to isCollectible: false for code running outside the editor, I'd be able to use Moq and finish porting everything over to 4.0 :D

@neikeq Hey again, sorry for bothering!

Any updates on this? I really want to make sure all of my packages are ready for 4.0 (and I have to be able to run my existing unit tests against my packages after migrating them to 4.0 to make sure they're passing). Any chance of a fix for this making its way into the next beta?

edit: it pains me to ping you, as I know how busy you are with this. If I should be pestering someone else, just let me know haha.

Sorry, for the delay. Can you check if the PR solves the issue?

@neikeq Hi! I have actually never built Godot from source (or the c# bindings, for that matter). I plan to give that a shot, but it may take me some time to figure out how to do all that. In any case, I'm confident what you've done will fix my issue since others who had built it with collectible: false had reported to me that it fixed the issue outside the editor.

Thank you so much 💯

A few extra notes while I'm thinking of it:

Over the last month, I've tried many other mocking libraries besides Moq with Godot 4 and have determined that any mocking library using Castle or even System.Reflection.Emit will break without collectible assembly support (basically, collectible assemblies thwart dynamic code generation because of a bug in System.Reflection.Emit). Almost all C# mocking libraries use dynamic code generation, at least as of late 2022 when I'm writing this.

Coincidentally, I have just found a potential work around (literally minutes before you opened that PR) by using LightMock.Generator (which itself uses LightMock) and confirmed that it works within the context of a collectible assembly since it uses a source generator to create mocks.

So, theoretically, if #67511 turns out to be an undesired change for other reasons, there is at least one solution within a collectible assembly context for creating mocks for use in game code unit tests. Granted, that project may not have the same clout or API as Moq, but it's still a viable solution.

Fixed by #67511.