GoKibitz doesn't render all symbols from SGF file

[archpaladin1]

In games where multiple spots on the goban are marked with squares or letters, GoKibitz does not render them all.

Check out https://gokibitz.com/kifu/NkpoO84p?path=25 (should be move 25). Compare with move 25 on the Go server: https://online-go.com/review/77270

Letters B & C are missing, and only one square appears instead of two.

I heard at some point that OGS doesn't always put out properly formatted SGF files, so perhaps this is the problem, but when I examine the file in a text editor it seems to look okay.

[traveller42]

I have verified the SGF file for that review.

It is not consistent with the specification. In particular, the same Property Type appears multiple times in the same move which is not permitted by the spec. Several SGF readers do handle this in a reasonable manner. The consistency checker located with the specification merges the values from the duplicate Move Properties as if they had been presented in the list form that they should have been in the first place.

This issue has been reported to, and acknowledged by, the OGS developers.

[levelonedev]

Thanks for the hard work guys! Should I keep this ticket open until OGS fixes the SGF rendering problem?

[traveller42]

I'd keep this open. The issue can come up again even after OGS has fixed their problem.

Existing files may get used later and there is always the possibility of other implementations creating flawed SGF.

If we merge the duplicates, this is a reasonable handling of an undefined condition.

[neagle]

Thanks for doing the research into this, @traveller42!

[neagle]

GoKibitz uses WGo.js as its SGF player. Without looking into the details, it might not be terribly difficult to update it to be able to handle this condition properly, so this could be a candidate for a pull request to that project. That said, it can be dangerous to start down the rabbit hole of dealing with non-valid SGF files: clients can start doing that in different ways, and conflict can emerge down the road. The best solution by far would be to get this resolved at the source.

[traveller42]

Thanks for the redirect. I'll follow-up there.

While I agree that the best solution is to fix things are the source, the spec does mention attempting to correct malformed properties.

[neagle]

@traveller42 That's a relevant point. Do you have a link to the point where the spec mentions that? I tried to look for it quickly this morning, but couldn't find it: it'd be nice to provide that if you open a ticket for WGo.js.

[traveller42]

It is on this page (http://www.red-bean.com/sgf/sgf4.html) in section 2.2.3.

I've reviewed WGo.js (your fork and the upstream). Using the included Player, I get the proper rendering of the board at the specified move from the SGF in the OP's review.

It is clearly not the same in GoKibitz. I am working through the internal representation in WGo. It might be that the node structure retains the duplicates and this is handled differently by WGo's Player and GoKibitz's Kifu.

I'm looking to run a few SGF's round-trip through the fromSGF and toSGF methods, and the related JSON methods.

If (as I currently suspect) the issue is in the WGo internal representation, I will open the issue there and included a Pull Request with my fix.

[neagle]

Thanks for the link! (That doc has name attributes on its link, so even though it's not obvious, you can get a link to a specific section: http://www.red-bean.com/sgf/sgf4.html#2.2.3)

Hmmmm: if WGo's out-of-the-box player handles things correctly, then it's possible there's something going on with the way I handle SGF files at some point. I'll try to look at the problem this evening and see what I can find.

Thanks for all your research on this, @traveller42.

[traveller42]

I did some more digging and I have the following information:

• If I run the SGF round trip the output had the SQ and LB properties collected per the spec.
• The markup properties exist separately in the Kifu object.

The node for move 25 in the original SGF: ;B[ic]SQ[mc]LB[id:B]LB[kd:C]SQ[md]LB[me:A]C[The group at N17 ended up dead. Playing at any of the other marked locations are probably better. Black can develop thickness to the outside and be pleased with the gain.]

The same node in the output from Kifu.toSgf() (also from WGo.SGF.parse(sgfString).toSgf()): ;B[ic]SQ[mc][md]LB[id:B][kd:C][me:A]C[The group at N17 ended up dead. Playing at any of the other marked locations are probably better. Black can develop thickness to the outside and be pleased with the gain.]

The JSON for the same node from Kifu.toJGO(): {"move":{"x":8,"y":2,"c":1},"markup":[{"x":12,"y":2,"type":"SQ"},{"x":8,"y":3,"type":"LB","text":"B"},{"x":10,"y":3,"type":"LB","text":"C"},{"x":12,"y":3,"type":"SQ"},{"x":12,"y":4,"type":"LB","text":"A"}],"comment":"The group at N17 ended up dead. Playing at any of the other marked locations are probably better. Black can develop thickness to the outside and be pleased with the gain."}