Skip to content

Possible off-by-one error in FastList.cs #59

Description

@elementarsenic

Hi, sorry if I'm misunderstanding or anything- I was trying to teach myself by reading the code, so I could eventually make a mod, and I found this.

public void RemoveAt(int index)
{
count--;
if (index == count - 1)
{
return;
}
for (int i = index; i < count; i++)
{
elements[i] = elements[i + 1];
}
}

Line 50 is what concerns me- it seems intended to skip the realignment of other elements if the element removed was the last element in the list- which makes sense because there would be nothing to realign. However, because count was already decremented in line 48, it actually seems to check if the element to be removed is the SECOND-to-last in the list. This would then result in the elements not being modified at all, but the count decreasing by one, which in effect actually removes the LAST element instead.

So when removing the last element, the method still realigns the empty values past it, which doesn't cause any issue to my knowledge except rendering the optimization pointless. When removing the second to last element, however, the wrong element is removed, which seems like it could be potentially disastrous. It seems like FastList is currently unused by the vanilla game, but it may still be worth fixing in case a modder tries to use it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions