-
-
Notifications
You must be signed in to change notification settings - Fork 29
Refactor event invocation #209
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
Conversation
|
The documentation preview is available at https://preview.netcord.dev/209. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors event invocation methods in the Gateway client to improve memory efficiency by avoiding unnecessary lambda closures. The changes convert captured lambdas to static lambdas with explicit parameter passing.
- Refactored
InvokeEventAsyncoverloads to accept raw data parameters and static lambda functions - Changed
JsonReadyEventArgs.Applicationproperty from nullable to non-nullable - Removed redundant
ApplicationFlagsproperty fromGatewayClient(now accessible viaReadyEventArgs) - Updated all event invocations throughout
GatewayClientandVoiceClientto use the new pattern
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| NetCord/Gateway/WebSocketClient.cs | Added new InvokeEventAsync overloads with raw data parameters and updated UpdateLatencyAsync to use new pattern; contains bugs in the 5-parameter overload (missing static modifier and incorrect method call) |
| NetCord/Gateway/Voice/VoiceClient.cs | Updated event invocations to use static lambdas with explicit parameter passing |
| NetCord/Gateway/JsonModels/EventArgs/JsonReadyEventArgs.cs | Changed Application property from nullable to non-nullable |
| NetCord/Gateway/GatewayClient.cs | Removed ApplicationFlags property, removed unused import, converted GetGuildId to static local function, and updated all event invocations to new pattern |
| NetCord/Gateway/EventArgs/ReadyEventArgs.cs | Removed null checks for Application property access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reordered generic constraints in `InvokeEventAsync` method signatures to ensure consistent ordering. Updated the `where TClient : WebSocketClient` constraint to appear before `where T : allows ref struct` and `where TRaw : allows ref struct`. These changes improve code readability and adhere to coding standards.
Refactored `InvokeEventAsync` and `InvokeEventWithDisposalAsync` calls in `GatewayClient` and `VoiceClient` classes to improve readability and maintainability: - Standardized parameter naming (`data` instead of `rawData`) across event handlers. - Simplified `CHANNEL_DELETE`, `THREAD_DELETE`, `GUILD_DELETE`, `GUILD_SCHEDULED_EVENT_DELETE`, and `STAGE_INSTANCE_DELETE` handlers in `GatewayClient` by using consistent variable names and tuple structures. - Updated `VoiceClient` to align parameter naming and improve clarity in `VoiceReceiveEventArgs` construction. These changes enhance code consistency and reduce complexity, making the codebase easier to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.