search

loomnetwork / unity-sdk

Loom DAppChain SDK for Unity3d
Other
121 stars 34 forks source link

Revert back to System.Numerics.BigInteger instead of Org.BouncyCastle.Math.BigInteger #27

Closed ZimM-LostPolygon closed 6 years ago

ZimM-LostPolygon commented 6 years ago

For EVM contracts, Unity SDK currently uses Org.BouncyCastle.Math.BigInteger instead of System.Numerics.BigInteger, since due to a Unity bug, System.Numerics is not referenced in generated C# projects. This is a major inconvenience, since Nethereum had to be modified to use Org.BouncyCastle.Math.BigInteger, and, to be honest, Org.BouncyCastle.Math.BigInteger is much worse compared to the native .NET System.Numerics.BigInteger in terms of interface and convenience.

However, I accidentally found out that Unity has a callback that is called after C# project are generated. This allows to add System.Numerics reference by trivially modifying the projects: https://gist.github.com/ZimM-LostPolygon/13c4a196f241072939d477998278948c

If we add this to the SDK, we won't have to maintain a fork of Nethereum that uses Org.BouncyCastle.Math.BigInteger, and everyone will be able to use System.Numerics.BigInteger, which is much nicer. The downside, obviously, is that the any existing C# code that uses Org.BouncyCastle.Math.BigInteger will break. This is unfortunate, but I believe using the native System.Numerics.BigInteger type is beneficial enough to justify that.

@enlight What do you think?

enlight commented 6 years ago

Does the original Nethereum use the real System.Numerics.BigInteger or do they have a customized version? If the Editor post-processor hack works reliably on Windows/Mac/Linux then I'd be in favor of switching to System.Numerics.BigInteger. Is that the only reason we have a Nethereum fork?

ZimM-LostPolygon commented 6 years ago

The original Nethereum uses the real System.Numerics.BigInteger type, the customized version was made by me. But no, that's not the only reason for the fork - its initial purpose was to change the namespace, so as to avoid conflicts. Also, the fork uses an older version of NewtonSoft.Json, since no AOT-friendly newer versions exists.

I'll do the check on macOS and Linux Unity editors, but generally, the modification is trivial enough to be reliable across the board.

ZimM-LostPolygon commented 6 years ago

@enlight Turned out, even though would happily accept the modified project, and it would use an assembly that uses System.Numerics just fine, it would still refuse to compile the code itself, so this approach with hacking the project files isn't working in the end. However... System.Numerics.dll is just 90 kB, and now I don't see any harm in just including it with the Loom SDK, at least until Unity fixes their bug with System.Numerics not being referenced by default.