Conversation
1. ForwardEntity in MultiEntity displays an exception 2. ForwardEntity in MultiEntity has extra `not null` text after it 3. Allow using ImageEntity in ForwardEntity in MultiMsg
There was a problem hiding this comment.
Pull Request Overview
This PR addresses several issues in the core messaging and resource upload components, including handling exceptions in ForwardEntity, removing extraneous text, and allowing ImageEntity usage in MultiMsg.
- Introduces a new efficient serializer utility method.
- Renames and refactors methods in MessagePacker and IMessageEntity to support alternate packing behaviors.
- Adjusts the ForwardEntity implementation to derive property values from MessageChain and improves uploader method signatures across multiple files.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Lagrange.Core/Utility/Extension/ProtoExt.cs | Adds a new SerializeToBytes extension method with aggressive inlining. |
| Lagrange.Core/Message/MessagePacker.cs | Refactors message packing method call from PackElement to PackFackElement. |
| Lagrange.Core/Message/IMessageEntity.cs | Introduces PackFackElement as a wrapper for existing behavior. |
| Lagrange.Core/Message/Entity/ForwardEntity.cs | Reworks property getters to use MessageChain and modifies packing logic. |
| Lagrange.Core/Internal/Service/Message/GetGroupMessageService.cs | Adds conditional payload check for retcode and handles group messages. |
| Lagrange.Core/Internal/Packets/Message/Action/SsoGetGroupMsgResponse.cs | Adds new proto members Retcode and Message for response handling. |
| Lagrange.Core/Internal/Context/Uploader/* | Updates uploader interfaces and implementations to use uid/uin instead of MessageChain. |
| Lagrange.Core/Internal/Context/Logic/Implementation/MessagingLogic.cs | Adjusts logic to process ForwardEntity resources correctly. |
| Lagrange.Core/Internal/Context/HighwayContext.cs | Integrates new uploader methods for group and private resource uploads. |
Comments suppressed due to low confidence (2)
Lagrange.Core/Message/MessagePacker.cs:90
- The method name 'PackFackElement' may be a misspelling. Consider renaming it to 'PackFakeElement' if that reflects its intended behavior.
message.Body?.RichText?.Elems.AddRange(entity.PackFackElement());
Lagrange.Core/Message/Entity/ForwardEntity.cs:72
- The hard-coded string 'not null' in the text element appears unintended and may lead to confusing output; consider removing or replacing it with a more meaningful value.
Str = "not null",
There was a problem hiding this comment.
Pull Request Overview
This PR addresses several issues related to ForwardEntity behavior and uploader consistency. Key changes include refactoring ForwardEntity to derive property values from a unified MessageChain, updating the packing methods (PackElement and PackFakeElement) to resolve exception issues and remove extraneous text, and modifying uploader interfaces and implementations for video, audio, and image entities.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Lagrange.Core/Utility/Extension/ProtoExt.cs | Added SerializeToBytes method to simplify serialization operations. |
| Lagrange.Core/Message/MessagePacker.cs | Updated to invoke PackFakeElement for fake message building. |
| Lagrange.Core/Message/IMessageEntity.cs | Introduced default implementation for PackFakeElement. |
| Lagrange.Core/Message/Entity/ForwardEntity.cs | Refactored property getters to use MessageChain and revised packing logic. |
| Lagrange.Core/Internal/Service/Message/GetGroupMessageService.cs | Enhanced response handling by checking Retcode and returning an empty chain when needed. |
| Lagrange.Core/Internal/Packets/Message/Action/SsoGetGroupMsgResponse.cs | Added Retcode and Message fields to the response body. |
| Lagrange.Core/Internal/Context/Uploader/*Uploader.cs | Modified uploader methods to use uid/uin parameters instead of MessageChain. |
| Lagrange.Core/Internal/Context/Uploader/IHighwayUploader.cs | Updated uploader interface to reflect the new signatures. |
| Lagrange.Core/Internal/Context/Logic/Implementation/MessagingLogic.cs | Added resolution logic for ForwardEntity and adjusted chain updates based on message type. |
| Lagrange.Core/Internal/Context/HighwayContext.cs | Updated resource upload calls and introduced separate methods for group and private uploads. |
| return true; | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Consider adding logging or clearer error handling in the non-zero Retcode branch to surface potential issues rather than silently returning an empty message chain.
| { | |
| { | |
| Logger.Warn($"Non-zero Retcode received: {payload.Body.Retcode}. Returning an empty message chain."); |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several issues around message forwarding and entity handling while updating serialization and API response handling. Key changes include:
- Introduction of a new method SerializeToBytes in ProtoExt.cs.
- Updates to ForwardEntity to use MessageChain for property access and adjustments to fake packing elements.
- API signature modifications in message services and uploader interfaces to incorporate retcode and explicit uid/uin parameters.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Lagrange.Core/Utility/Extension/ProtoExt.cs | Added SerializeToBytes helper method for byte array generation. |
| Lagrange.Core/Message/MessagePacker.cs | Revised element packing to support fake elements. |
| Lagrange.Core/Message/IMessageEntity.cs | Added default PackFakeElement implementation. |
| Lagrange.Core/Message/Entity/ForwardEntity.cs | Refactored property usage and packing/unpacking logic for forwarded messages. |
| Lagrange.Core/Internal/Service/Message/GetGroupMessageService.cs | Modified chain parsing using retcode. |
| Lagrange.Core/Internal/Service/Message/GetC2cMessageService.cs | Similarly updated result construction with retcode. |
| Lagrange.Core/Internal/Packets/Message/Action/SsoGetGroupMsgResponse.cs Lagrange.Core/Internal/Packets/Message/Action/SsoGetC2cMsgResponse.cs |
Updated message responses to include retcode and message string. |
| Lagrange.Core/Internal/Event/Message/GetC2cMessageEvent.cs | Adjusted constructor parameters to accommodate retcode. |
| Lagrange.Core/Internal/Context/Uploader/*Uploader.cs Lagrange.Core/Internal/Context/Uploader/IHighwayUploader.cs |
Changed uploading method signatures to use uid/uin. |
| Lagrange.Core/Internal/Context/Logic/Implementation/MessagingLogic.cs | Added upload resource handling for ForwardEntity. |
| Lagrange.Core/Internal/Context/HighwayContext.cs | Refactored resource uploading calls with updated signatures. |
Comments suppressed due to low confidence (1)
Lagrange.Core/Message/Entity/ForwardEntity.cs:72
- The hard-coded string "not null" used for the Text element may be unintentional. Consider removing or replacing it with a more descriptive message that accurately reflects the intended behavior.
Str = "not null",
not nulltext after it