Take NosCore.Packets 21.1.0 and drop the manual mlintro escape - #2339
Conversation
The serializer now encodes spaces as "^" itself: for any string that is not the last field, for strings nested in a sub-packet, and for the few fields that declare EscapeSpaces because the client wants them encoded wherever they sit. MlintroPacket.Intro is one of those, so the handler passes the message through as typed. MlEditPacketHandlerTests asserted the caret on the packet object, which is no longer where it appears. It now checks the object carries the message as typed and pins "mlintro Test^Test" on the wire. MinilandEntranceHandler keeps its substitution. It writes the message onto MsgPacket, which five other call sites send unescaped, so the field cannot declare EscapeSpaces without mangling those.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe ChangesMiniland message encoding
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change keeps miniland messages in their original form while preserving caret escaping on the client wire format. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Consumer half of NosCoreIO/NosCore.Packets#499, released as 21.1.0.
What the serializer now does
Spaces are encoded as
^by the serializer in three cases, so callers stop doing it:pinitmate names were going out as10|Joyeux Mouton|0and breaking the packet aboveEscapeSpaces, for text the client wants encoded wherever it sits —mlintro, the miniland message onmlinfobr, the family message onginfoWhat changes here
MlobjPacketHandlerpasses the message through as typed;MlintroPacket.IntrodeclaresEscapeSpaces.MlEditPacketHandlerTestsasserted the caret on the packet object, which is no longer where it appears. It now checks the object carries the message as typed and pinsmlintro Test^Teston the wire, so the escaping is still covered end to end rather than just deleted.What deliberately stays
MinilandEntranceHandlerkeeps itsReplace(' ', '^'). It writes ontoMsgPacket, and five other call sites send that same field unescaped —"Your respawn location has been changed."among them — so the field cannot declareEscapeSpaceswithout mangling them. Making this one serializer-owned would need a second class on themsgheader withAllowDuplicateHeader, which is worse than one line in the handler.Fixed for free
Miniland.GenerateMlinfobr()sentMinilandMessageraw whileMinilandEntranceHandlerescaped the same string ontomsga few lines away, so a spaced miniland message rendered differently depending on which packet carried it.EscapeSpacesonMlInfoBrPacketsettles that without a change here.GInfoPacket.FamilyMessagegets the same treatment, which matters for #2283.Tests
Full solution builds against 21.1.0 after a cache clear. GameObject 532, PacketHandlers 410, Parser 139, Database 11, Core 10, WebApi 17 — all green under the CI filter.
Summary by CodeRabbit
Bug Fixes
Tests