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

Fix AIM needs to be closed multiple times when moving all inventory/worn items to ground #47602

Merged
merged 1 commit into from
Feb 20, 2021

Conversation

Qrox
Copy link
Contributor

@Qrox Qrox commented Feb 19, 2021

Summary

Bugfixes "Fix AIM needs to be closed multiple times when moving all inventory/worn items to ground"

Purpose of change

When moving all inventory/worn items to ground, AIM needs to be closed multiple times to be actually closed.

Describe the solution

Only call do_return_entry once when AIM is about to be automatically closed due to pending activity, and do not call do_return_entry if there's no pending activity.

In addition, some adjustments are made to relevant code to clarify code and fix several bugs:

  1. In advanced_inventory::move_all_items, entry is now directly assigned with the next value instead of incremented at the beginning. This fixes a bug where only the items in the first (south-west) vehicle square is moved when moving all surrounding items.
  2. on_out_of_scope is used to restore the all items location after selecting a specific location for moving, in case the code returns early.
    2.1. The copy/move constructor/assignment operators of on_out_of_scope and restore_on_out_of_scope are marked as deleted, because copying and moving are not well-defined for these classes.
  3. A no enough room query is shown before moving inventory/worn items to the ground/a vehicle, if there is not enough room at the destination square, to be consistent with moving items between ground/vehicle squares.
  4. Moving items on the ground to worn items is not implemented in AIM, so it is now disabled.
  5. AIM now always restore the previously selected vehicle or ground square, unless there isn't a vehicle square or only one of the vehicle and ground squares contains items, in which cases it selects the ground square or square with items respectively.

Testing

  1. Tested moving items between different source and destination tiles. Items were moved correctly and AIM only needed to be closed once. Moving all surrounding items also correctly moved items in vehicles.
  2. Tested the "CLOSE_ADV_INV" setting and AIM was closed after moving all surrounding items when the setting was on, and not closed when the setting was off.
  3. Tested moving items to a vehicle tile without enough storage when the destination square was "all". The selected square was restored to "all" regardless of whether the query about lacking storage was confirmed or canceled.
  4. Tried to move inventory items to a vehicle tile without enough storage, and a query about lacking storage was displayed.
  5. Tried to move ground items to worn items, and it showed a warning popup instead of moving items to the inventory.
  6. Reopened AIM with different combination of nearby ground/vehicle tiles with/without items, and the selected squares did not change unless there wasn't a vehicle square or only one of the vehicle and ground squares contained items.

Additional context

This can be merged for 0.F while #45243 is under feature freeze.

@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones labels Feb 19, 2021
@ZhilkinSerg ZhilkinSerg merged commit 1460d28 into CleverRaven:master Feb 20, 2021
@Qrox Qrox deleted the aim-exit branch February 20, 2021 13:59
andrei8l added a commit to andrei8l/Cataclysm-DDA that referenced this pull request Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants