feat(quest): add QuestProgressS6PlugIn for Season 6 client compatibility#793
feat(quest): add QuestProgressS6PlugIn for Season 6 client compatibility#793noelfelipe wants to merge 2 commits into
Conversation
## Problem
The default QuestProgressPlugIn sends a 251-byte C1 F6 0C packet in the
Season 9 format (QuestProgress struct). When a Season 6 GMO client
receives this packet, it interprets it as PMSG_NPC_QUESTEXP_INFO and
attempts to parse the trailing data as NPC_QUESTEXP_REQUEST_INFO[5] and
NPC_QUESTEXP_REWARD_INFO[5] arrays. Because the packet layout does not
match what the S6 client expects, it reads garbage byte counts, loops
out of bounds over an internal array, and disconnects.
## Root Cause
OpenMU uses a single QuestProgressPlugIn with no client-version guard.
The packet it produces matches the Season 9+ protocol (C1 header,
QuestProgress / QuestState struct layout). The Season 6 client speaks a
different, older protocol for the same opcode (F6 0C):
Client S6 expected layout (C2 packet, 272 bytes):
PWMSG_HEADER (4 bytes) : C2 sizeH sizeL 0xF6
SubCode (1 byte) : 0x0C
RequestCount (1 byte)
RewardCount (1 byte)
RandCount (1 byte)
QuestIndex (4 bytes) : (Group << 16) | Number, little-endian
5 x NPC_QUESTEXP_REQUEST_INFO (28 bytes each, MSVC default alignment):
QUEST_REQUEST_TYPE [+0] BYTE (1) + pad (1)
m_wIndex [+2] WORD (2)
m_dwValue [+4] DWORD (4) required count / level
m_wCurValue [+8] DWORD (4) current progress
m_byItemInfo[15] [+12] BYTE (15)
struct pad [+27] BYTE (1)
5 x NPC_QUESTEXP_REWARD_INFO (24 bytes each, MSVC default alignment):
QUEST_REWARD_TYPE [+0] BYTE (1) + pad (1)
m_wIndex [+2] WORD (2)
m_dwValue [+4] DWORD (4) amount
m_byItemInfo[15] [+8] BYTE (15)
struct pad [+23] BYTE (1)
## Solution
Add QuestProgressS6PlugIn decorated with [MinimumClient(6, 0)] so that
the ViewPlugInContainer selects it over the undecorated default plugin
for any Season 6 client. For Season 9+ clients the existing
QuestProgressExtendedPlugIn (MinimumClient 106.3) takes precedence.
The plugin builds the 272-byte packet manually using BinaryPrimitives
for endian-safe writes and maps OpenMU domain objects to S6 wire types:
OpenMU QuestMonsterKillRequirement -> QUEST_REQUEST_MONSTER (1)
- Monster.Number -> m_wIndex
- MinimumNumber -> m_dwValue
- KillCount (state) -> m_wCurValue
OpenMU QuestItemRequirement -> QUEST_REQUEST_ITEM (3)
- Item type (group<<9|num)-> m_wIndex
- MinimumNumber -> m_dwValue
- inventory count -> m_wCurValue
- serialized item data -> m_byItemInfo[15]
OpenMU QuestReward (Experience) -> QUEST_REWARD_EXP (1)
OpenMU QuestReward (Money) -> QUEST_REWARD_ZEN (2)
OpenMU QuestReward (Item) -> QUEST_REWARD_ITEM (4)
- item type -> m_wIndex
- Value -> m_dwValue
- serialized item data -> m_byItemInfo[15]
## Affected files
src/GameServer/RemoteView/Quest/QuestProgressS6PlugIn.cs (new)
No existing files were modified. The original QuestProgressPlugIn
can be disabled via the PlugInConfiguration table in the database
(set IsActive = false for GUID 5D2B7F90-FEFA-4889-B339-D64512471613)
to prevent it from competing with this plugin on servers that only
serve Season 6 clients.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new plugin, QuestProgressS6PlugIn, to resolve compatibility issues between the OpenMU server and Season 6 clients. Previously, the server sent quest progress packets in a format expected by newer clients, causing Season 6 clients to misinterpret the data and disconnect. The new plugin ensures that the server sends the correct, legacy-compatible packet structure when a Season 6 client is detected. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces QuestProgressS6PlugIn, a Season 6-compatible implementation of IQuestProgressPlugIn that formats and sends quest progress data to the client. The feedback highlights several potential NullReferenceException risks, specifically when accessing kill.Monster, itemReq.Item, and virtual collections like RequiredMonsterKills, RequiredItems, and Rewards. Additionally, it is recommended to replace the placeholder GUID with a unique, randomly generated one to prevent registration conflicts.
- Replace placeholder GUID with randomly generated one (B10A7CBD-...) - Use null-safe collection access: RequiredMonsterKills?.ToList() ?? new() - Use null-safe monster dereference: kill.Monster?.Number ?? 0 - Guard against null Item in WriteItemReqSlot with early return pattern
|
Sorry, but this doesn't look right yet:
|
2 participants
Problem
The default QuestProgressPlugIn sends a 251-byte C1 F6 0C packet in the Season 9 format (QuestProgress struct). When a Season 6 GMO client receives this packet, it interprets it as PMSG_NPC_QUESTEXP_INFO and attempts to parse the trailing data as NPC_QUESTEXP_REQUEST_INFO[5] and NPC_QUESTEXP_REWARD_INFO[5] arrays. Because the packet layout does not match what the S6 client expects, it reads garbage byte counts, loops out of bounds over an internal array, and disconnects.
Root Cause
OpenMU uses a single QuestProgressPlugIn with no client-version guard. The packet it produces matches the Season 9+ protocol (C1 header, QuestProgress / QuestState struct layout). The Season 6 client speaks a different, older protocol for the same opcode (F6 0C):
Client S6 expected layout (C2 packet, 272 bytes):
\
PWMSG_HEADER (4 bytes) : C2 sizeH sizeL 0xF6
SubCode (1 byte) : 0x0C
RequestCount (1 byte)
RewardCount (1 byte)
RandCount (1 byte)
QuestIndex (4 bytes) : (Group << 16) | Number, little-endian
5 x NPC_QUESTEXP_REQUEST_INFO (28 bytes each, MSVC default alignment):
QUEST_REQUEST_TYPE [+0] BYTE (1) + pad (1)
m_wIndex [+2] WORD (2)
m_dwValue [+4] DWORD (4) required count / level
m_wCurValue [+8] DWORD (4) current progress
m_byItemInfo[15] [+12] BYTE (15)
struct pad [+27] BYTE (1)
5 x NPC_QUESTEXP_REWARD_INFO (24 bytes each, MSVC default alignment):
QUEST_REWARD_TYPE [+0] BYTE (1) + pad (1)
m_wIndex [+2] WORD (2)
m_dwValue [+4] DWORD (4) amount
m_byItemInfo[15] [+8] BYTE (15)
struct pad [+23] BYTE (1)
\\
Solution
Add QuestProgressS6PlugIn decorated with [MinimumClient(6, 0)] so that the ViewPlugInContainer selects it over the undecorated default plugin for any Season 6 client. For Season 9+ clients the existing QuestProgressExtendedPlugIn (MinimumClient 106.3) takes precedence.
The plugin builds the 272-byte packet manually using BinaryPrimitives for endian-safe writes and maps OpenMU domain objects to S6 wire types:
Affected files
No existing files were modified. The original QuestProgressPlugIn can be disabled via the PlugInConfiguration table in the database (IsActive = false for GUID 5D2B7F90-FEFA-4889-B339-D64512471613) on servers that only serve Season 6 clients.