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

Converted all projects to .NET 6 #139

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

LennardF1989
Copy link

@LennardF1989 LennardF1989 commented Aug 14, 2023

Following up on #137 - I took the first step and convert all projects. It was somewhat painless. Tested the ConverterApp with BG3 files so far and everything still works and extracts (if you take a slightly older save BEFORE the latest update).

This would probably need some more testing with other game files as well. Leaving it as a draft as it shapes up.

For everyone who feels like it: please make PR's on my PR-branch to help shape this up.

Will have to restore some of the project targets that were in the original files, but other than that, it's mostly testing and fixing what needs fixing.

Could probably be (partially) combined with work done on #107 to get proper Linux support for the libs.

This is a VERY verbatim conversion, only fixing what was required to get it to compile. In most spots, code could benefit from a cleanup: removing unused usings, reformatting, using Path.Combine to concatenate paths instead of +, etc.

@LennardF1989
Copy link
Author

Ran into a few problems with Bags of Sorting - so made some tweaks to the project files:

  • Everything that references LSLib is now x64, not AnyCPU.
  • Properly converted the LSLibNative to the DotNetCore CLR
  • Added support for exporting LSLib as a NuGet package (including all references it needs to work properly, even "hidden" ones .NET won't directly tell you about due to LSLibNative)

The last one is probably the most important to make distribution and usage in other projects as painless as possible. Be warned though: simply don't try to use "produce single file" and "trim unused code". It doesn't play nicely with LSLibNative in particular.

Other than that, it's probably a good idea to replace Newtonsoft.Json with System.Net.Json to get rid of another dependency. Some portions of LSLibNative can probably be rewritten as managed.

@clemarescx
Copy link

clemarescx commented Sep 9, 2023

Translated from native to managed, the CRC32 hash computation gives:

CRC32 C++ to C# translation
using System.Runtime.InteropServices;

namespace Native;

public static class Crc32
{
    public static uint Compute(ReadOnlySpan<byte> input, uint previousCrc32)
    {
        if (input.Length == 0)
        {
            return previousCrc32;
        }

        var pin = MemoryMarshal.Cast<byte, uint>(input);

        var current = 0;
        int length = input.Length;
        uint crc = ~previousCrc32;

        InitCrc32LookupTable();
        const int increment = 8;

        while (length >= increment)
        {
            uint one = pin[current++] ^ crc;
            uint two = pin[current++];
            crc = _crc32Lookup[7, one & 0xFF]
                ^ _crc32Lookup[6, (one >> 8) & 0xFF]
                ^ _crc32Lookup[5, (one >> 16) & 0xFF]
                ^ _crc32Lookup[4, one >> 24]
                ^ _crc32Lookup[3, two & 0xFF]
                ^ _crc32Lookup[2, (two >> 8) & 0xFF]
                ^ _crc32Lookup[1, (two >> 16) & 0xFF]
                ^ _crc32Lookup[0, two >> 24];

            length -= increment;
        }

        var rest = input[^length..];
        current = 0;

        while (length-- != 0)
        {
            crc = (crc >> 8) ^ _crc32Lookup[0, (crc & 0xFF) ^ rest[current++]];
        }

        return ~crc;
    }

    private static uint[,] _crc32Lookup;

    private static void InitCrc32LookupTable()
    {
        if (_crc32Lookup != null)
        {
            return;
        }

        _crc32Lookup = new uint[8, 0x100];
        const uint xorMagic = 0xEDB8_8320;
        for (uint i = 0; i <= 0xFF; i++)
        {
            uint crc = i;
            for (uint j = 0; j < 8; j++)
            {
                uint shift = (crc >> 1);
                var xorMem = -(int)(crc & 1) & xorMagic;
                crc = (uint)(shift ^ xorMem);
            }

            _crc32Lookup[0, i] = crc;
        }

        for (uint i = 0; i <= 0xFF; i++)
        {
            _crc32Lookup[1, i] = (_crc32Lookup[0, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[0, i] & 0xFF];
            _crc32Lookup[2, i] = (_crc32Lookup[1, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[1, i] & 0xFF];
            _crc32Lookup[3, i] = (_crc32Lookup[2, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[2, i] & 0xFF];
            _crc32Lookup[4, i] = (_crc32Lookup[3, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[3, i] & 0xFF];
            _crc32Lookup[5, i] = (_crc32Lookup[4, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[4, i] & 0xFF];
            _crc32Lookup[6, i] = (_crc32Lookup[5, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[5, i] & 0xFF];
            _crc32Lookup[7, i] = (_crc32Lookup[6, i] >> 8) ^ _crc32Lookup[0, _crc32Lookup[6, i] & 0xFF];
        }
    }
}

Both this and the "de-CLR-ized" version of the native CRC32 (CompilerExplorer link) hash the byte array [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a] to 622876539.

Alternatively, if we don't mind adding Nuget packages, Microsoft's System.IO.Hashing has a Crc32 hasher that provides the same result with:

static uint ComputeCrc32Microsoft(byte[] hashMe)
{
    var res = System.IO.Hashing.Crc32.Hash(hashMe);
    return BitConverter.ToUInt32(res);
}

I have not benchmarked the differences, but I'd assume Microsoft's solution is at least as fast as the verbatim C# translation of the C++ code.

@LennardF1989
Copy link
Author

LennardF1989 commented Sep 9, 2023

Been using this as part of my project for BG3 and have yet to experience any issues other than the ones I have already fixed.

As for the CRC: I'm all for just pulling in System.IO.Hashing, why re-invent the wheel? Especially since it's a Microsoft-maintained package.

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.

2 participants