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

SocketIDをイベントとして取得できるように修正 #10

Merged
merged 38 commits into from
Jan 24, 2024

Conversation

sasuke9410
Copy link
Contributor

@sasuke9410 sasuke9410 commented Dec 13, 2023

何の変更を加えましたか?

  • ホスト/クライアントの開始時に自身のSocketIDを取得できるようにしました
  • 他のクライアントの接続/切断時に、接続/切断したクライアントのSocketIDを取得できるようにしました
  • C#側のコードで、切断してもホスト/他のクライアントからは数秒接続し続けているように見えていたのを、DisconnectAsyncを呼ぶことですぐに切断されるように修正しました(12823c0)
  • 開始時の自身のSocketIDと他のクライアントの接続/切断時のSocketIDを使用するように、サンプルを修正しました
  • Hookのエラーハンドリングの修正時にWebGL側で非同期処理のHookがawaitされない状態になっていたためawaitされるように修正しました
  • Hookのエラーハンドリングの修正時に、WebGL側の非同期処理確認用のサンプルが追加されていなかったため追加しました

何を確認しましたか?

実装

  • Frameworkの誤った使い方にすぐに気づけるように、無効な引数や無効なメソッド呼び出しに対するチェックが入っていることを確認しました
  • Framework実行時の動きが分かるように、ログ(Error/Warn/Info/Debug)を出力していることを確認しました
  • 静的解析で問題が見つからないことを確認しました
  • フレームワーク利用者が使うAPI(主にprivate以外)に C# ドキュメントを記述しました

テスト

  • 全ての自動テストが成功することを確認しました
    • 自動テストが存在しないためスキップ
  • テストカバレッジが100%になることを確認しました
  • サンプルがあるものはサンプルが動作することを確認しました

変更影響

  • GuideのReleaseページに変更内容が追加されることを確認しました
  • GuideのModuleページ(機能ページ)に変更が反映されることを確認しました
  • GuideのLearningページに変更が反映されることを確認しました
    • 変更なし
  • Sample Applicationに変更が反映されることを確認しました
    • 変更なし

レビュアーへのメッセージ

@sasuke9410 sasuke9410 changed the base branch from develop to add-error-handling December 13, 2023 11:55
@sasuke9410 sasuke9410 changed the base branch from add-error-handling to develop December 14, 2023 01:16
@sasuke9410 sasuke9410 changed the base branch from develop to add-error-handling December 14, 2023 01:18
@sasuke9410 sasuke9410 changed the base branch from add-error-handling to develop December 14, 2023 01:23
@sasuke9410 sasuke9410 changed the base branch from develop to add-error-handling December 14, 2023 01:24
@sasuke9410 sasuke9410 changed the base branch from add-error-handling to develop December 14, 2023 01:26
@sasuke9410 sasuke9410 changed the base branch from develop to add-error-handling December 14, 2023 07:51
@sasuke9410 sasuke9410 changed the title 自身のClientIDを取得できるように修正 ClientIDをイベントとして取得できるように修正 Dec 14, 2023
@sus-yoshikane-t sus-yoshikane-t force-pushed the add-id-and-user-connection-event branch from 0054285 to 57bf002 Compare December 14, 2023 09:19
@sasuke9410 sasuke9410 changed the base branch from add-error-handling to develop December 14, 2023 09:46
@sasuke9410 sasuke9410 changed the title ClientIDをイベントとして取得できるように修正 SocketIDをイベントとして取得できるように修正 Dec 14, 2023
@sus-yoshikane-t sus-yoshikane-t force-pushed the add-id-and-user-connection-event branch from 4c4136d to b1b148b Compare December 14, 2023 10:51
@mahaweilu
Copy link
Contributor

  • C#側のコードで、即時で切断するメソッドを呼んでいなかったのを修正しました
  • WebGL側での非同期処理のHookの動作がエラーハンドリングの修正時に変わってしまっていたため修正しました
  • サンプルでエラーハンドリング周りの処理を確認する部分が一部抜けていたため、サンプルを修正しました

レビュアーへのメッセージ

この3つの説明が読むだけで分からなったので修正お願いしたいです。主に以下の観点です。

  • ピンポイントとなる修正箇所をこちらのPR説明の1行目と同じようにcommitリンクを添付してもよさそうです
  • 現状developの違い
    • 例えば、「即時で切断するメソッドを呼んでいなかった」で何の事象が発生するかです

@sus-yoshikane-t sus-yoshikane-t force-pushed the add-id-and-user-connection-event branch from 37577e6 to 1529bc7 Compare January 10, 2024 01:44
@sus-yoshikane-t sus-yoshikane-t merged commit ed76057 into develop Jan 24, 2024
1 check passed
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.

5 participants