Use .NET Core 3.1 #96
Conversation
FrEee.WinForms/post-build.bat
Outdated
| @@ -4,15 +4,15 @@ for /f "tokens=1,* delims= " %%a in ("%*") do set ALL_BUT_FIRST=%%b | |||
|
|
|||
| echo Copying assets to %ALL_BUT_FIRST%\bin\%1\net472 | |||
There was a problem hiding this comment.
Again we are mentioning a nonexistent net472 folder.
There was a problem hiding this comment.
And fixed again. :)
|
So it looks like there are still 2 remaining issues:
|
|
It turns out that error code 1 appears on first build after manually deleting bin/ and obj/ folders from FrEee.Tests. The normal rebuild wasn't enough - only after removing directories by hand I was able to reproduce it. Luckily, this entire post-build step isn't really necessary - I was able to use new MSBuild capabilities introduced in .NET Core to encode moving those folders in the csproj file itself, so I've dropped the post-build.bat from test project. The WinForms project probably could do without it too, but I can't parse what the current batch file does to the scripts, and it doesn't give me any problems, so I've left it for now. You might want to look into removing it later on. As for the ValueInjector, there is the warning that I receive when building the solution: |
|
Oh, thanks for fixing the build! :) So if ValueInjecter failed then lots of things would break, because it's used when serializing and deserializing data, and also in instantiating templates for objects such as ships. (if I'm not mistaken, I haven't touched that code in a while). So if you are able to play a few turns of the game without it crashing, I guess it is compatible! |
|
I tried running the game from your branch and I noticed it is unable to load any existing files such as game setups because mscorlib does not exist in .NET Core. You might have to add some code to |
|
I had to replace mscorlib with System.Private.CoreLib everywhere, but now quickstart actually works, so it seems to have gone well. |
|
Hmm, now I get this error when I try to run a quickstart game: System.InvalidOperationException: 'Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.' at System.ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported() |
|
dotnet/runtime#26868 - this explains the concurrency error we were getting |
|
@KarbonKitty you should be able to fix that concurrency error by changing the internal implementation of |
|
I've replaced the Dictionary with ConcurrentDictionary (it needed replacing Remove with TryRemove method, but it seemed a trivial change). It doesn't crash on me now, but it also didn't crash on me before, so it'd be great if you could take a try and make sure it works for you now. As an aside, it seems that ConcurrentDictionary has a set of methods that do the same thing that the SafeDictionary does (AddOrUpdate, GetOrAdd etc.), so it might be prudent to go with .NET Core class in the future. |
|
That seems to have worked, thanks! |
Ports FrEee.Core, FrEee.Tests and FrEee.WinForms to .NET Core 3.1.
In Tests, MSTest was replaced with NUnit, some tests were commented out, and there are 10 more tests failing (there were 12 before, and I'm unclear on whether those failures are legitimate). Review of the tests is necessary before going forward.
Otherwise, there were surprisingly few issues; one major outstanding issue is the fact that ValueInjector in the version used by FrEee (2.3.3) is .NET Framework - based, and new version seems to contain some breaking changes that I was incapable of resolving right away; review of possible upgrade of that version would be a good idea too.
Some TODOs were added in code; most important touches the post-build.bat/post-build.sh duality; in .NET Core the build system should be capable of selecting correct file per platform, and xbuild.sh hack should be unnecessary. In the meantime, post-build step is hardcoded using post-build.bat file.