# What is the intent of remove\_mission\_items?

**URL:** <https://discourse.cataclysmdda.org/t/what-is-the-intent-of-remove-mission-items/877>\
**Category:** The Garage\
**Created:** [April 18, 2013, 3:13am UTC](https://discourse.cataclysmdda.org/t/what-is-the-intent-of-remove-mission-items/877 "2013-04-18T03:13:35Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![Soron](https://sea2.discourse-cdn.com/flex016/user_avatar/discourse.cataclysmdda.org/soron/32/885_2.png) [@Soron](https://discourse.cataclysmdda.org/u/Soron)\
**Post date:** [April 18, 2013, 3:13am UTC](https://discourse.cataclysmdda.org/t/what-is-the-intent-of-remove-mission-items/877/1 "2013-04-18T03:13:35Z")

</div>

Okay, I’m currently cleaning up inventory code, which in some cases means I’m moving stuff out of player:: and into inventory:: (e.g., calculating inventory weights). One of those cases is some code from remove\_mission\_items.

Just one issue: it doesn’t seem to be written sanely, and I can’t tell if it’s supposed to remove just one item, or all items.

Here’s the code that was there before I started modifying things:

```auto
void player::remove_mission_items(int mission_id)
{
 if (mission_id == -1)
  return;
 if (weapon.mission_id == mission_id)
  remove_weapon();
 else {
  for (int i = 0; i < weapon.contents.size(); i++) {
   if (weapon.contents[i].mission_id == mission_id)
    remove_weapon();
  }
 }
 for (int i = 0; i < inv.size(); i++) {
  for (int j = 0; j < inv.stack_at(i).size() && j > 0; j++) {
   if (inv.stack_at(i)[j].mission_id == mission_id) {
    if (inv.stack_at(i).size() == 1) {
     inv.remove_item(i, j);
     i--;
     j = -999;
    } else {
     inv.remove_item(i, j);
     j--;
    }
   } else {
    bool rem = false;
    for (int k = 0; !rem && k < inv.stack_at(i)[j].contents.size(); k++) {
     if (inv.stack_at(i)[j].contents[k].mission_id == mission_id) {
      if (inv.stack_at(i).size() == 1) {
       inv.remove_item(i, j);
       i--;
       j = 0;
      } else {
       inv.remove_item(i, j);
       j--;
      }
      rem = true;
     }
    }
   }
  }
 }
}
```

Now, from what I can tell, we have the following possible outcomes:

[ul][li]If we find a stack of matching items, we abort.[/li]  
[li]However, if we find a container whose contents match, we remove that container, and do not abort.  
[list]  
[li]If the container is the sole item in the stack, we remove it, and then proceed to check the next item.[/li]  
[li]If the container is not the sole item in the stack, we remove it, and then skip over the next item, continuing on to check the item AFTER.[/li]  
[/list][/li][/ul]

So, I’m not sure whether we’re supposed to be removing all matching items, or a stack, or just one. If anyone else knows what the intended behavior is, I’d appreciate hearing it. As it stands, I’m probably going to intentionally reproduce the current behavior, because I don’t want to introduce any additional bugs while I’m doing my current refactor.

---

<div class="post-metadata">

**Author:** ![Soron](https://sea2.discourse-cdn.com/flex016/user_avatar/discourse.cataclysmdda.org/soron/32/885_2.png) [@Soron](https://discourse.cataclysmdda.org/u/Soron)\
**Post date:** [April 18, 2013, 3:37am UTC](https://discourse.cataclysmdda.org/t/what-is-the-intent-of-remove-mission-items/877/2 "2013-04-18T03:37:34Z")

</div>

Actually, never mind, I figured it out: it removes items until it finds a relevant item that’s not stacked with anything. So… I think it would be safe to remove all relevant items from the first stack that matches.