OldUnreal / Unreal-testing

OU 227 testing
Other
57 stars 0 forks source link

UnrealShare.ArbitItem incorrectly removes item classes #320

Closed Masterkent closed 1 year ago

Masterkent commented 1 year ago

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--).

Marco888 commented 1 year ago

Implemented the fix.