loomnetwork / unity-sdk

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

Refactored Address to not be a partial class of Protobuf Address class #23

Closed ZimM-LostPolygon closed 6 years ago

ZimM-LostPolygon commented 6 years ago

Address was the only Protobuf class from the SDK that was supposed to be used by end-users. Generally, this should not happen - Protobuf stuff is too low-level, and there's a reason why Protobuf classes are sealed - they are not supposed to be extended in any way. It was also quite annoying to see Protobuf-related methods in autocomplete when working with addresses.

This PR basically adds a nicer Address struct and hides all Protobuf stuff away from end users' eyes.

Noteworthy:

enlight commented 6 years ago

Very nice! Did you test AddressJsonConverter in WebGL?

ZimM-LostPolygon commented 6 years ago

I didn't. What's specific about it for WebGL?

On Sat, Jun 23, 2018, 18:56 Vadim Macagon notifications@github.com wrote:

Very nice! Did you test AddressJsonConverter in WebGL?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/loomnetwork/unity3d-sdk/pull/23#issuecomment-399692886, or mute the thread https://github.com/notifications/unsubscribe-auth/AA4rJr3mFDvKWmXphSpCsm6ixmNvA1t9ks5t_nMygaJpZM4U02O9 .

enlight commented 6 years ago

Just the usual WebGL issues that crop up when symbols are stripped during the build. This looks safe enough though I suppose.

ZimM-LostPolygon commented 6 years ago

@enlight Ah. It's not just WebGL then, also iOS, and, optionally, Android and Desktop. But yes, this should be fine.