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

Fixed type fetching in the part passed to MessagePack. #73

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

yukimakura
Copy link

@yukimakura yukimakura commented Jan 29, 2022

Fix type fetching in the part passed to MessagePack.

IRemoteRequestHandler with multiple IAsyncRequestHandler interfaces was throwing a deserialization exception when called over TCP or a named pipe. The cause seems to be the assumption that there is only one IAsyncRequestHandler.

↓ Specifically, the following way of having the interface was supported.

public interface ICustomRequestHandle : IAsyncRequestHandler<string, string>, IAsyncRequestHandler<Caller2Request, Caller2Response>
{
}

public class RPCHogehoge: ICustomRequestHandle 
{
...

Sorry if I'm using the wrong premise in the first place.

This is my first pull request. Please approve it.

MessagePipe is a very powerful library, and I am using it effectively. I'm looking forward to the future development of this library.


MessagePackに渡す部分の型取得修正

IRemoteRequestHandlerにてTCPまたは名前付きパイプで呼び出すと、
複数の IAsyncRequestHandlerインターフェイスを持つクラスにてデシリアライズ例外が発生しました。
原因は IAsyncRequestHandlerを一つしか持たない前提の処理になっていたようです。

↓具体的には以下のようなインターフェイスの持ち方に対応しました。

public interface ICustomRequestHandle : IAsyncRequestHandler<string, string>, IAsyncRequestHandler<Caller2Request, Caller2Response>
{
}

public class RPCHogehoge: ICustomRequestHandle 
{
...

そもそも前提の使い方を間違えていたらごめんなさい。

初めてのプルリクエストです。
フォーマットや内容など至らぬ点ございますが、
承認よろしくお願いいたします。

MessagePipe非常に強力なライブラリで有効活用させていただいております。
これからもこのライブラリの発展に期待しております。

@neuecc
Copy link
Member

neuecc commented Feb 1, 2022

ありがとうございます!
良いと思います!

@neuecc neuecc merged commit 7aa2fc8 into Cysharp:master Feb 1, 2022
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.

3 participants