Skip to content
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

Added multi-build for net472 (client) and netstandard 2.0 (server) #1972

Closed
wants to merge 13 commits into from

Conversation

Measurity
Copy link
Collaborator

@Measurity Measurity commented Jan 16, 2023

  • Find alternative serializer for .netstandard 2.0, BinaryPack supports 2.1 only and Nitrox.BinaryPack for net472 does not work with 2.0 as is. Issue found by TwinBuilderOne, breaking change in .NET 5+ caused wrong union order for packets due to culture string compare: https://learn.microsoft.com/en-us/dotnet/standard/base-types/string-comparison-net-5-plus
  • Ensure build process is correct (NitroxLauncher depends on Server.exe, but can not reference it as a project because launcher runs on net472 at the moment.)

@Measurity Measurity force-pushed the netstandard branch 4 times, most recently from f40239f to 6caf658 Compare January 21, 2023 00:04
@Measurity Measurity marked this pull request as ready for review January 21, 2023 00:19
Measurity and others added 4 commits January 21, 2023 01:34
Issue with netstandard and Nitrox.BinaryPack wasn't related. Instead a breaking change with string comparison in new .NET 5+ runtimes caused wrong serializer type order.

Co-authored-by: TwinBuilderOne <jackreynolds9128@gmail.com>
These tests have been broken for a long time and soon we'll have source generators that help fix serializing issues.
The unit tester needs a .netstandard 2.0 reference to run tests properly without warnings.
@Measurity
Copy link
Collaborator Author

This PR is ready for review

Copy link
Collaborator

@tornac1234 tornac1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some new warnings happening after this PR modifications:

6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(31,28,31,58): warning CA1416: This call site is reachable on all platforms. 'RotateFlipType.RotateNoneFlipY' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(31,9,31,59): warning CA1416: This call site is reachable on all platforms. 'Image.RotateFlip(RotateFlipType)' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(29,98,29,125): warning CA1416: This call site is reachable on all platforms. 'PixelFormat.Format32bppArgb' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(29,26,30,80): warning CA1416: This call site is reachable on all platforms. 'Bitmap' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\AddressablesTools\Catalog\SerializedObjectDecoder.cs(64,28,64,64): warning CA1416: This call site is reachable on all platforms. 'Type.GetTypeFromCLSID(Guid)' is only supported on: 'windows'.

Directory.Build.props Outdated Show resolved Hide resolved
Directory.Build.props Outdated Show resolved Hide resolved
Nitrox.BuildTool/Nitrox.BuildTool.csproj Show resolved Hide resolved
Nitrox.BuildTool/Nitrox.BuildTool.csproj Show resolved Hide resolved
NitroxLauncher/NitroxLauncher.csproj Show resolved Hide resolved
@tornac1234
Copy link
Collaborator

Also breaks tests

@Rosentti
Copy link
Contributor

For the servers to work on linux, System.Drawing needs to be replaced with SkiaSharp or ImageSharp.
If you just want the warnings fixed, you can add IFs around the code that uses System.Drawing. This isn't the most ideal solution though, as it does add a bunch of if statements into the code.

@Measurity
Copy link
Collaborator Author

Measurity commented Jan 21, 2023

For the servers to work on linux, System.Drawing ...

This is something I'll look into in the Linux branch once this is merged.

@Measurity
Copy link
Collaborator Author

Measurity commented Jan 21, 2023

There are some new warnings happening after this PR modifications:

6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(31,28,31,58): warning CA1416: This call site is reachable on all platforms. 'RotateFlipType.RotateNoneFlipY' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(31,9,31,59): warning CA1416: This call site is reachable on all platforms. 'Image.RotateFlip(RotateFlipType)' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(29,98,29,125): warning CA1416: This call site is reachable on all platforms. 'PixelFormat.Format32bppArgb' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\Parsers\RandomStartParser.cs(29,26,30,80): warning CA1416: This call site is reachable on all platforms. 'Bitmap' is only supported on: 'windows'.
6>[...]\NitroxServer-Subnautica\Resources\AddressablesTools\Catalog\SerializedObjectDecoder.cs(64,28,64,64): warning CA1416: This call site is reachable on all platforms. 'Type.GetTypeFromCLSID(Guid)' is only supported on: 'windows'.

Those warnings are new because the code can now run on Linux but it still uses some windows only code. It is something I will fix in the Linux branch.

I've set the runtime to windows only until the code is ready for linux (will happen in linux branch).

Measurity and others added 4 commits January 21, 2023 17:22
…uticaUwePrefabFactory.cs

Co-authored-by: rootcan <tortornac@gmail.com>
Co-authored-by: rootcan <tortornac@gmail.com>
Co-authored-by: rootcan <tortornac@gmail.com>
@Measurity Measurity force-pushed the netstandard branch 2 times, most recently from 5ae267a to e7b366a Compare January 21, 2023 16:34
@Measurity
Copy link
Collaborator Author

I think I solved those issues now Tornac 🙏 .

Parallel builds will still show warnings on file locks, but it should work regardless. I will migrate the launcher over to .net 7 in linux branch which will make it better compatible with normal MSBuild flow (using project references).

…d2.0

NitroxClient and NitroxPatcher should be loaded as netstandard2.0 for testing only (Subnautica does not fully support netstandard2.0 APIs)
@Measurity
Copy link
Collaborator Author

Changes in this PR were merged into #1848

@Measurity Measurity closed this Jan 23, 2023
@Measurity Measurity deleted the netstandard branch January 23, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants