removecargo() game-breaking bug

[mnalis]

utils.pas:removecargo(item) if given item which does not exist, will remove some other random item!

That can break the game - in my case, Pirate was supposed to remove shunt drive (which wasn't there because when Titarians joined Kendar it disappeared - another bug?), so it instead removed homing device, making the game unplayable.

procedure removecargo(item: integer);
var j: integer;
begin
 j:=1;
 while (j<250) and (ship.cargo[j]<>item) do inc(j);
 if (j<251) and (ship.numcargo[j]>254) then j:=251;
 if j>251 then exit;
 assert (j<=250);
 dec(ship.numcargo[j]);
 if ship.numcargo[j]=0 then ship.cargo[j]:=0;
end;

This is buggy on several layers!

• should check on =item, and not j
• j can never be 251, only 250 (which is also valid cargo number)
• and that >254 check break stuff. Shouldn't it be >1 anyway since we're doing dec() and not inc()?

On related note:

• Also, check possible bug with 254 in addcargo2() (similar problem possible as one fixed in addcargo() in 1f8de2fb088dc46a35e2eb853e521516487a7d1e)
• Also, check Titarians are supposed to remove shunt drive (and how it handles with Pirate event)

[mnalis]

Titarians are supposed to remove shunt drive; but I guess it was expected to happen after the pirates take (and we later recover) shunt drive. In weird.pas:event():

   47   : begin            { titarian like shuntdrive }
             removecargo(ID_ART_SHUNT_DRIVE);
             addlog(47);
          end;