Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Items] Overhaul Item Handin System #4512

Draft
wants to merge 44 commits into
base: master
Choose a base branch
from

Conversation

Akkadius
Copy link
Member

@Akkadius Akkadius commented Oct 14, 2024

Description

This is a very long overdue overhaul to a overly complex system. That aims to solve many issues and simplify item handins.

Why

  • Instead of having separately maintained hand-in logic in Perl and Lua, they both pass-through existing plugins to the source and are handled entirely source side. Reducing bugs, defects and allowing us to far more easily add features.
  • Player event records sometimes don't get caught under certain circumstances, now every hand-in no matter whether its handled in quest or not gets captured
  • Item handin gets handled the same in both Perl and Lua
  • Money handin gets handled the same in both Perl and Lua
  • Support multi-quest natively in the source for NPC's who have it enabled
  • Heavily simplify item handin logic
  • Add comprehensive logging to debug deeper level handin issues

Catch-all for returning items to players

  • Item loss would occur when a player hands in to an NPC with an empty EVENT_TRADE
  • Item loss would occur when a player accidentally hands in to a pet instead of the intended NPC
  • Item loss would occur if a script failed to add a "return_items" plugin equivalent with hand-in code
  • NPCs with no proper check_handin/return_items would eat items.
  • NPCs were returning a new item, not the previous item instance (items with augments). Items are simply return as their original instance, with all augments, attuned properties, serial number etc.
  • Augments were eaten in Perl when doing handins because we were summoning by item ID instead of the item instance itself.

Associated Quest Plugin Changes

ProjectEQ/projecteqquests#1403

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing

image

image

image

Checklist

  • I have tested my changes
  • I have performed a self-review of my code. Ensuring variables, functions and methods are named in a human-readable way, comments are added only where naming of variables, functions and methods can't give enough context.
  • I have made corresponding changes to the documentation (if applicable, if not delete this line)
  • I own the changes of my code and take responsibility for the potential issues that occur

@Akkadius
Copy link
Member Author

Akkadius commented Oct 14, 2024

This is not ready for testing yet. Few things to wrap up.

When ready, we will do extensive testing and get the help of several to hit it.

@Valorith
Copy link
Contributor

Nice!

@Akkadius Akkadius force-pushed the akkadius-kingly/overhaul-item-handin branch from fc0122b to 19affb6 Compare October 20, 2024 21:19
@Kinglykrab

This comment was marked as outdated.

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.

3 participants