paviad / GoSharp

A C# class library for the game of Go (Igo, Weiqi, Baduk)
MIT License
23 stars 13 forks source link

Cannot have two branches both beginning with pass move #16

Closed nerai closed 6 years ago

nerai commented 6 years ago

The Game class stores moves in a dictionary. If two moves map to the same key, this leads to an exception.

Passes use a singleton instance as key, so they are affected.

Regular moves use custom Point objects, which override GetHashCode but not equals, so they are not affected. I'm not sure if this is an unintended side effect.

Example SGF:

(;GM[1]FF[4]CA[UTF-8]AP[Kibble:0]ST[2]
SZ[19]
;B[]
(;W[]C[first])
(;W[]C[second])
)
nerai commented 6 years ago

(Writing a patch right now)

nerai commented 6 years ago

I see several possible approaches and would like to ask for clarification.

  1. I found that the dictionary is not accessed via key. This means we could just as well use a normal list of tuples. However, tuples are not available in .NET 3.5, we need 4.5. In 4.7, we could even use named tuples. So can we upgrade the project to 4.5? That would be the easiest way.

  2. Make a separate list of pass moves in addition to the dictionary of regular moves. Use both lists where needed. This is a horrible hack.

  3. In addition to idea 1, we could also move the key into the Game object entirely. It seems this is possible, but would require some refactoring. I am not sure, but I think the areas affected need some cleanup anyway (Game constructors, Game.Move property in serialization). This may be the best in the long run in addition to 1., but takes more time (i.e. I'd only submit it later), and I might need some assistance.

paviad commented 6 years ago

You are right on point 1. I converted the dictionary to a list of Variation objects which contain two properties, a Move and a Game.

Commit 4c7a2d5 handles this