Repository navigation
Conversation
Until now, QuestCompletionAction completed the quest and dropped an item reward which didn't fit into the inventory at the player's feet, where anyone could take it after the owner time; a money reward over the money limit was lost.
Now the quest isn't completed while the item rewards don't fit, counting the space of the handed in items as free, or while the money rewards would exceed the money limit. Nothing is taken. The server answers with the quest completion response (F6 0D), whose field IsQuestCompleted becomes the result value which the Season 6 client reads: 0 not completed, 1 completed, 2 zen limit reached, 3 inventory full. For 2 and 3 the client shows its message ("You have reached your Zen limit", "Inventory is full") and disables the button to complete the quest. In the Season 6 data the legacy quests have no item or money rewards (all 338 item and 9 money rewards belong to the newer quests); their response (C1 A2) has no such result, so a refusal sends nothing for them.
IStorage.CanAddItems tells if items fit into a storage, including its extensions, after other items were removed, without changing it. It places them in the order of CheckInvSpace, like AddItemAsync does.
The packet docs were updated by hand, because the markdown generation needs Node.js.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 1 minor |
🟢 Metrics 41 complexity · 0 duplication
Metric Results Complexity 41 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Part of #1475 (part A, the second of three PRs; independent of #1487).
QuestCompletionActioncompletes a quest even when its rewards don't fit: an item reward which finds no space in the inventory is dropped at the player's feet, where anyone can take it after the owner time, and a money reward over the money limit is lost. The Season 6 client already knows the answer for this case: its quest completion response (F6 0D) carries a result byte, 0 = not completed, 1 = completed, 2 = zen limit reached, 3 = inventory full. For 2 and 3 it shows its message and disables the button to complete the quest (ReceiveQuestCompleteResultin MuMain'sWSclient.cpp). OpenMU modelled the byte as a boolean and only ever senttrue.What
IsQuestCompletedofQuestCompletionResponse(F6 0D) becomesResult, an enumQuestCompletionResultwith the values the client reads. The generated code and tests are generated again; the markdown page of the packet is updated by hand, because its generation needs Node.js, which I don't have here.IQuestCompletionResponsePlugIngetsQuestCompletionRefusedAsync(quest, refusal)with aQuestCompletionRefusal(InventoryFull,ZenLimitReached); the remote view sends result 3 or 2. The legacy quests (group 0, C1 A2) have no such result, so nothing is sent for them; in the Season 6 data they have no item or money rewards (all 338 item and 9 money rewards belong to the newer quests).IStorage.CanAddItems(items, removedItems)tells if items would fit into a storage, including its extensions, after other items were removed, without changing it. It places them in the order ofCheckInvSpace, likeAddItemAsyncdoes.Not changed: when the server refuses for other reasons (items or kills missing), it still sends nothing. The client does nothing on result 0 either, so answering it wouldn't change anything there.
Testing
dotnet buildanddotnet test --no-buildwith-p:ci=true:MUnique.OpenMU.Tests: 2891 passed, 0 failedMUnique.OpenMU.Network.Packets.Tests: 717 passed, 0 failedQuestRewardSpaceTest(7 tests):CanAddItemswith an empty storage, items which don't fit together, removed items which make room, extensions; a quest completed when the reward fits into the place of the handed in item; refused with a full inventory (nothing taken, quest still active, refusal sent); refused when the money reward would exceed the limit. The two refusal tests fail without the change.quest2Dwwith "Clearing a Path to Icarus (Dark Wizard)" active (60/60 kills, reward Jewel of Chaos) and a full inventory. Clicking OK at the Shadow Phantom Soldier: the client shows "Inventory is full." and disables the button; the quest stays active. After freeing one cell: "Jewel of Chaos Obtained", the jewel is in the freed cell and the quest window closes. The zen limit was only tested by the unit test.Found while testing: #1489 (merchant store items which no longer fit) and #1490 (the quest window shows item rewards as "Item: x 0").