Skip to content
This repository was archived by the owner on Mar 11, 2021. It is now read-only.

Show numbers on moves branched from main line #852

Merged
merged 7 commits into from
Jul 24, 2019

Conversation

amj
Copy link
Contributor

@amj amj commented Jul 18, 2019

All this seems to work ok. It's a little weird -- the annotation layer always copies in its annotations from the underlying Position object, so i ended up making these annotations at the time the new Positions are created and just toggling their visibility. It's not a great solution but it seems to work ok. Happy to think about refactors that would make this make more sense.

Switching the last-move indicator to unicode seems to work and also to be aligned correctly :)

The fix on board.ts seems like a new one because of type information added to the context object? I don't think i changed the original behavior but maybe i did?

@amj amj requested a review from tommadams July 18, 2019 22:55
@amj
Copy link
Contributor Author

amj commented Jul 18, 2019

fixes #849

@amj
Copy link
Contributor Author

amj commented Jul 18, 2019

oh hmm, this actually won't work really well if there's a ko in the variation. Maybe don't review it just yet XD

@amj amj removed the request for review from tommadams July 18, 2019 22:59
@amj
Copy link
Contributor Author

amj commented Jul 23, 2019

Ok, i added some duplication check to the annotation code in position.ts. The annotation layer should probably be keeping a map keyed by position instead of a flat array, but maybe that's for next time. Ready for review here

@amj amj requested a review from tommadams July 23, 2019 19:15
Copy link
Contributor

@tommadams tommadams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't finish reviewing yet, I've got to head out but here are a few initial comments.

Thanks for doing this

@amj amj merged commit c2d478c into tensorflow:master Jul 24, 2019
@amj amj deleted the label-divergence branch July 24, 2019 19:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants