Skip to content

Conversation

@Tiernan-Alderman
Copy link
Contributor

@Tiernan-Alderman Tiernan-Alderman commented May 31, 2024

Changes made: Modified inventorycomponent.cpp and inventorycomponent.h to include a queue to keep track of item ids when items are being sold to a vendor. This also comes with two new methods, which accomplish this.

What was fixed: Buyback inventory size of27 will be maintained and there will always be an empty slot, at the end reopening the oldest item slot which is the first slot in the inventory, which was not live accurate and prevented players from being able to buy back any further items past the 27 item limit.

Way things were tested: Selling 27+ items, of multiple different types, some with stack sizes. It functions as intended, emptying the oldest slot in the vendor inventory when items are being sold. These changes should not affect other portions of inventories as this will only be applied when the inventory a vendor buyback type.

Thanks for the patience on this one

@EmosewaMC EmosewaMC changed the title Buyback inventory overflow glitch fix fix: buyback inventory live accuracy May 31, 2024
Copy link
Collaborator

@EmosewaMC EmosewaMC left a comment

Choose a reason for hiding this comment

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

Logic is fine overall, but where stuff is living has some interesting decisions based on how you decided to implement the logic for the buyback inventory. We will convene over discord at some point and discuss how to design the API in a more robust way.

All logs in this PR should be removed when finished unless they are reporting an error, alongside the comment about if there is a bug if thats ok, in this case no it is not because it is an easily fixed bug and this feature is not critical enough to replace a bug with another one

@Tiernan-Alderman Tiernan-Alderman marked this pull request as ready for review June 2, 2024 03:51
@DarwinAnim8or
Copy link
Member

Any updates on this? Is it ready for review?

@jadebenn
Copy link
Collaborator

Seconding the question about merge readiness. I'm coming back from a period of long inactivity, and I wouldn't mind helping bringing this back to a ready-to-merge state.

@EmosewaMC
Copy link
Collaborator

there was some odd behavior with how items were popped from the inventory that I was working with them to resolve (would result in a funky deletion order)

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.

4 participants