ujh/iomrascalai — pull request #30 by TisButMe (2014-06-25 08:45:53+00:00)
Title: This should be an actual fix to the bug #19, and maybe to #29 ?

I think I figured out what the issue was. This also passes our unit test, but could you try it with gnugo & GoGUI (don't have those on hand right now...) ?

The problem was:
-> When removing more than one chain at once, the id of the second chain to remove might change because of the first chain being removed from the storing vector, and we didn't account for that.

Why did HashSet solve it: Pure luck. The set had an effective order inversed, which meant it removed the chains in the order that didn't impact the ids of chains to be later removed.

2 solutions:
-> (Adopted) Use a Vec, sort and dedup it, and then count the number of removed chains so that we update the id "pointer" to reflect the changes in the chains vector.
-> Use a Vec, sort, dedup and then reverse, so that we're removing chains later in the chains vector first, not impacting the actual ids of the other chains to remove. I thought this made for a less clear code, as well as maybe not being as good performance wise (extra reverse instead of just incrementing a var), which is why I adopted the other option.