OldUnreal/Unreal-testing

UnrealShare.ArbitItem incorrectly removes item classes

Closed this issue · 1 comments

In 227k 2023-04-04, UnrealShare.ArbitItem.CheckReplacement.BeginState uses the following for loop:

		for (i=0; i< numItems; i++)
		{
			// spawn item
			currentItem = spawn(SpawnItem[i], Owner, Tag, Location, Rotation);
			//if (currentItem != None) Level.Game.IsRelevant(currentItem);
			if (bDebug) log("CURRENT ITEM ===== "$i@CurrentItem,'ARBITITEM');
			if ((currentItem == None) || (currentItem.bDeleteMe))
			{
				// item was replaced
				currentItem = findReplacedItem();
				if (currentItem == none) // item was either destroyed or replaced by a non-Item
					removeItem(i);
				else // enter the class of the new item in the original's place
					SpawnItem[i] = currentItem.class;
			}
			if (currentItem != None) currentItem.destroy();
		}

In case if removeItem(i) is called, i is still incremented by means of i++, despite the fact that the next item with index i + 1 was moved to the position i, so this next item is not checked then.

A possible resolution could be replacing expression removeItem(i) with removeItem(i--).

Implemented the fix.