New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ObjectCreator assembly resolving on Windows. #13190

Merged
merged 2 commits into from Apr 26, 2017

Conversation

Projects
None yet
4 participants
@pchote
Member

pchote commented Apr 24, 2017

This fixes an issue that prevented out-of-tree compiled dlls from loading on Windows/.NET.

It appears that the assembly resolution on Windows is lazier than mono, and in this specific case does not try to resolve the Mods.Common dependency until after we have removed our custom assembly resolver.

This then leads to the following exception:

Could not load file or assembly 'OpenRA.Mods.Common, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' or one of its dependencies. The system cannot find the file specified.

I don't understand why it fails here but works for our main mod dlls. The monodis --assemblyref references to OpenRA.Game and OpenRA.Mods.Common are the same for both. In any case, keeping our ResolveAssembly override around until we dispose the ObjectCreator seems to fix the issue.

Testcase:

  • Check out https://github.com/OpenRA/OpenRAModTemplate/tree/wip on Windows
  • Compile the engine and OpenRA.Mods.Example project
  • Run the mod using the launch-game.cmd script and watch it die
  • Change the submodule reference to point to this PR
  • Recompile
  • Run the mod using the launch-game.cmd script and watch it run

Behaviour under mono should not change.

I guess that this explains why third party mods have always insisted on / resorted to hacks to put their dll projects in the main code tree, but I don't understand why it hasn't been reported or seriously discussed as a legitimate engine bug before now.

Requesting reviews from:

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Apr 25, 2017

Contributor

This is past above my head at the moment and I don't have time to review this properly, but this sounds like the reason why crashes like OpenRA/ra2#334 (comment) (yes, this was reported on OSX, but the end result is suspiciously similar) happened?

In any case, I never heard this bug myself, my insistence on the "hack to put the dll in the main code tree" is simply personal preference because I hated building against the compiled binaries in VS (and didn't even gave that a try after Linuxswitch) since that prevented me jumping from one code snappet to another through Go To Definition.

Contributor

GraionDilach commented Apr 25, 2017

This is past above my head at the moment and I don't have time to review this properly, but this sounds like the reason why crashes like OpenRA/ra2#334 (comment) (yes, this was reported on OSX, but the end result is suspiciously similar) happened?

In any case, I never heard this bug myself, my insistence on the "hack to put the dll in the main code tree" is simply personal preference because I hated building against the compiled binaries in VS (and didn't even gave that a try after Linuxswitch) since that prevented me jumping from one code snappet to another through Go To Definition.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Apr 25, 2017

Member

since that prevented me jumping from one code snappet to another through Go To Definition.

FYI you don't need the code to be in-tree to do this, and I plan to have the mod template support this. If you want to continue this discussion lets move it to IRC to avoid side-tracking this PR.

Member

pchote commented Apr 25, 2017

since that prevented me jumping from one code snappet to another through Go To Definition.

FYI you don't need the code to be in-tree to do this, and I plan to have the mod template support this. If you want to continue this discussion lets move it to IRC to avoid side-tracking this PR.

@RoosterDragon

The description of the issue you provide here is sound. On Windows, assemblies are loaded super lazily so the custom assembly resolution would indeed be removed before any of them were actually loaded. Keeping the assembly resolution active during the lifetime of the ObjectCreator is the way to fix that.

Show outdated Hide outdated OpenRA.Game/ObjectCreator.cs
@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Apr 25, 2017

Member

Updated.

Member

pchote commented Apr 25, 2017

Updated.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Apr 26, 2017

Contributor

I don't remember whether I've run into this (I probably did at some point), but I can confirm via the testcase that the fix works, so 👍

Contributor

reaperrr commented Apr 26, 2017

I don't remember whether I've run into this (I probably did at some point), but I can confirm via the testcase that the fix works, so 👍

@reaperrr reaperrr merged commit 539ed67 into OpenRA:bleed Apr 26, 2017

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr
Contributor

reaperrr commented Apr 26, 2017

@pchote pchote deleted the pchote:object-weirdness branch May 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment