Skip to content

bugfix: prevent quest item pickup by players without active quest#821

Open
eduardosmaniotto wants to merge 9 commits into
MUnique:masterfrom
eduardosmaniotto:bugfix/item-pickup
Open

bugfix: prevent quest item pickup by players without active quest#821
eduardosmaniotto wants to merge 9 commits into
MUnique:masterfrom
eduardosmaniotto:bugfix/item-pickup

Conversation

@eduardosmaniotto

@eduardosmaniotto eduardosmaniotto commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #761

Bug: Quest items can be picked up by party members without active quest

Problem: When a monster drops a quest item (e.g., promotion quest items like Scroll of Emperor), ALL party members are set as owners of the dropped item. The only guard — IsBoundToCharacter — checks whether the picker is an owner, which passes for all party members. This means characters without the active quest can pick up and hoard quest items.

Root cause: AttackableNpcBase.cs:444 assigns killer.Party?.PartyList.AsEnumerable() as owners, and DroppedItem.TryPickUpByAsync() only checks ownership, not quest eligibility.

Changes

PickupItemAction.cs (+22 lines):

  • New PlayerHasActiveQuestForItem helper — scans Character.QuestStates for any active quest whose RequiredItems match the item definition and level
  • Quest eligibility check: if item is IsBoundToCharacter and player has no active quest requiring it → shows "This item doesn't belong to you." and rejects
  • Ownership check: if player isn't an owner and the 10s owner-priority window is active → same message

DroppedItem.cs (+11 lines):

  • IsPlayerAnOwner changed from private to public so PickupItemAction can query it
  • Added IsOwnerPickupPriorityActive property exposing the 10-second owner-priority window

PlayerMessage.resx / PlayerMessage.Designer.cs (+12 lines):

  • Added ItemDoesNotBelongToYou = "This item doesn't belong to you."

Behavior

Scenario Result
Player already has 1 copy of quest item (limit=1) "Limit reached" (existing check, still fires first)
Player without active quest picks up quest item "This item doesn't belong to you."
Both party members have active quest, same item drops First-come-first-served (existing race behavior)
Non-party player tries to steal regular item within 10s window "This item doesn't belong to you."
After 10s, regular item is free for anyone Existing behavior preserved
dont-belong dont-belong-pt limit-reach

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces ownership and quest-binding restrictions when picking up dropped items. It adds checks to ensure players cannot pick up items bound to other characters unless they have an active quest for them, and respects the owner-pickup priority period. A review comment suggests adding a defensive null check on RequiredItems within the quest check logic to prevent a potential NullReferenceException.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/GameLogic/PlayerActions/Items/PickupItemAction.cs
@ze-dom

ze-dom commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

After 10s, item is free for anyone | Existing behavior preserved

Why not keep it locked to the quest player until it disappears? Otherwise the item will end up blocked in another player's inventory (can't sell, store or trade).

@eduardosmaniotto

eduardosmaniotto commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

After 10s, item is free for anyone | Existing behavior preserved

Why not keep it locked to the quest player until it disappears? Otherwise the item will end up blocked in another player's inventory (can't sell, store or trade).

That's only for regular items. Quest items will not be able to be picked up after the 10s window. I updated the description to make it clearer; currently if you try to steal a regular item, there is no message to the player. This PR fixes that too.

@sven-n

sven-n commented Jul 10, 2026

Copy link
Copy Markdown
Member

Thanks, I'll merge it, but I have one hint: The code now implicates that every dropped item which is bound to a character is a quest item, too 😄

@eduardosmaniotto eduardosmaniotto marked this pull request as draft July 10, 2026 20:17
@eduardosmaniotto

Copy link
Copy Markdown
Contributor Author

Thanks, I'll merge it, but I have one hint: The code now implicates that every dropped item which is bound to a character is a quest item, too 😄

I could create a IsQuestItem in ItemDefinition. It would be a simple check during runtime. What do you think?

@sven-n

sven-n commented Jul 10, 2026

Copy link
Copy Markdown
Member

Makes sense, it simplifies the code 😄

@eduardosmaniotto eduardosmaniotto marked this pull request as ready for review July 10, 2026 21:23
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, 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 addresses a bug where party members without an active quest could pick up and hoard quest items dropped by monsters. By introducing a specific quest item flag and updating the pickup validation logic, the system now enforces that only eligible characters can acquire these items, while maintaining existing owner-priority mechanics for other loot.

Highlights

  • Quest Item Eligibility: Introduced an IsQuestItem flag to ItemDefinition to distinguish quest items from other bound items, ensuring only players with an active quest can pick them up.
  • Pickup Logic Improvements: Updated PickupItemAction to include a helper method that validates if a player has an active quest for a specific item before allowing the pickup.
  • Owner Priority Exposure: Exposed IsPlayerAnOwner and added IsOwnerPickupPriorityActive to DroppedItem to allow for better validation during the 10-second item protection window.
  • Data Migration: Added a database migration and a data initialization plugin to flag existing quest items correctly in the system.
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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new IsQuestItem flag to ItemDefinition to restrict quest item pickups to characters with matching active quests, rather than relying solely on character-bound checks. It updates the pickup logic, adds database migrations, introduces an update plugin to flag existing quest items, and adds a localized message for ineligible pickups. Feedback suggests simplifying the LINQ query in PlayerHasActiveQuestForItem using OfType<QuestDefinition>() to remove null-forgiving operators, and removing the unnecessary async keyword from the synchronous ApplyAsync method in the update plugin.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/GameLogic/PlayerActions/Items/PickupItemAction.cs
Comment thread src/Persistence/Initialization/Updates/AddIsQuestItemFlagPlugIn.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quest item pick up by party member without a promotion quest

3 participants