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.

Sign up for GitHub

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

Jump to bottom

Inventory: Handle removeItem leftovers #8884

Merged
merged 1 commit into from Sep 2, 2019
Merged

Inventory: Handle removeItem leftovers #8884

merged 1 commit into from Sep 2, 2019

Conversation

SmallJoker
Copy link
Member

Fixes #8883

New output:

2019-09-01 09:45:11: [Server]: Starting inventory:
2019-09-01 09:45:11: [Server]: 1 default:wood 50
2019-09-01 09:45:11: [Server]: 2 default:wood 50
2019-09-01 09:45:11: [Server]: 3 default:wood 50
2019-09-01 09:45:11: [Server]: 4 default:wood 50
2019-09-01 09:45:11: [Server]: Normal stack max is 99
2019-09-01 09:45:11: [Server]: Removing: default:wood 100
2019-09-01 09:45:11: [Server]: Removed: default:wood 100
2019-09-01 09:45:11: [Server]: Ending inventory:
2019-09-01 09:45:11: [Server]: 1 default:wood 50
2019-09-01 09:45:11: [Server]: 2 default:wood 50

To do

This PR is Ready for Review.

How to test

  1. Take the code from InvRef's remove_item(listname, stack) method is giving wrong results when stack is larger than stack_max #8883 entirely, override the chest definition
  2. Fill in enough stacks
  3. Punch the testing node
  4. The removed count must be 100 and matches with the leftover items in the inventory.

@SmallJoker SmallJoker added the Bugfix 🐛 PRs that fix a bug label Sep 1, 2019
sfan5
sfan5 approved these changes Sep 1, 2019
View reviewed changes
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

LGTM

ClobberXD
ClobberXD reviewed Sep 2, 2019
View reviewed changes
src/inventory.cpp
@@ -680,7 +680,11 @@ ItemStack InventoryList::removeItem(const ItemStack &item)
for (auto i = m_items.rbegin(); i != m_items.rend(); ++i) {
if (i->name == item.name) {
u32 still_to_remove = item.count - removed.count;
removed.addItem(i->takeItem(still_to_remove), m_itemdef);
ItemStack leftover = removed.addItem(i->takeItem(still_to_remove),
m_itemdef);
Copy link
Contributor

Choose a reason for hiding this comment

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

Continuation lines only require one extra tab of indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

But two are OK too

Copy link
Contributor

Choose a reason for hiding this comment

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

One is what our code-style demands, though...

@SmallJoker SmallJoker merged commit f3acdd3 into minetest:master Sep 2, 2019
@SmallJoker SmallJoker deleted the itemstack_leftover branch July 5, 2020 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClobberXD ClobberXD ClobberXD left review comments

@sfan5 sfan5 sfan5 approved these changes

Assignees
No one assigned
Labels
Bugfix 🐛 PRs that fix a bug One approval ✅ ◻️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InvRef's remove_item(listname, stack) method is giving wrong results when stack is larger than stack_max
3 participants
@SmallJoker @sfan5 @ClobberXD