What is the intent of remove_mission_items?

Game Talk The Garage
1

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:

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.

2

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.