Use mouse to jump to any node(position) in variation window

[OlivierBlanvillain]

Reopens #374

[featurecat]

Olivier let's also include this in 0.6?

[OlivierBlanvillain]

@featurecat Sure, it just needs to be reviewed, it's next on my queue!

[OlivierBlanvillain]

Since the value k is not after this loop completes I would turn it into a local variable, like you did below with m. Same comments apply for the loops below.

[phiming]

k is working variable, it's c style.

[OlivierBlanvillain]

It looks like you copy the two loops above, do you think there is a way to avoid that duplication?

[phiming]

more details? I dont understand

[OlivierBlanvillain]

@phiming Here is my review. I've given you push access to my fork so that you can continue working on this branch if it makes things more convenient.

[phiming]

Thanks for your so careful review.
Wow, If I had known there is such strict rules to follow, I dare not commit anything. :-)
I just want to make things workable and simple. Anyway, since we have started it, let's finish it. I will reply one by one later

[phiming]

Thanks again, after all, we are volunteers. I will try best to help finish it. But since china's national day is over, I am available only at night. Sorry for some delay sometimes

[phiming]

I changed code just now. Adopt suggestions from above discussion. Long variable names, list instead of array, correct typo.

I push it to my homepage https://github.com/phiming/lizzie:oliver
I hope it will get approved but let me know if any question.

[OlivierBlanvillain]

Wow, If I had known there is such strict rules to follow, I dare not commit anything. :-)
I just want to make things workable and simple. Anyway, since we have started it, let's finish it.

@phiming Thanks for addressing my comments. I'm sorry if the review felt overwhelming, my intent here is simply to help making Lizzie a good piece of software. Trying to deeply understand and review other people changes to the code base is one was to get there.

Just to make things clear: there are no strict rules, all of this is just my opinion. And I think this PR needs more work before being mergeable. Don't feel obliged to work on it because you started, do as you please. Something that people often don't talk about is that for experienced people it can often take less time to re-implement something them-self than to help a contributor to meet their quality standard thought a dept review.

With that out of the way, I have two additional, higher level comments on your PR.

1. In jumpToAnyPosition why not simply walk all the way up to the root node, then all the way down to the targetNode? If that's possible, I think the current implementation is unnecessarily complicated by trying to optimize the path from the source node to the target node.

2. Most of the lines in inSubtree and inVariationTree are copy pasted from draw and drawTree, which is unacceptable. My suggestion would be to refactor both methods to pull out the tree traversal logic into a standalone method that could then be used both for drawing the variation tree and to identify the node to jump to in jumpVariationTree.

[phiming]

@OlivierBlanvillain
I understand that, as a code lover, you wish every line of code perfect. Although it's a little surprised you are not co-author of lizzie, I still appreciate your hard working. Thanks for making lizzie useful.

As I said, I have started it. But now it has been beyond my original purpose, to do some hack and let variation tree node clickable like in sabaki, and make lizzie easier. Maybe it's also a little bit beyond my ability. I am not a java programmer.

About two comments you mentioned,
1, Suppose that move number is over 200, and I click the next node in the tree, it has to go back to the root and then go to the target node according to your algorithm, which is high cost because every move will send command to leelaz, totally sending 400 commands to leelaz. But if we calculate before moving, it will send only one command to leelaz.
2, I still can't find a good way to refactor like that, there are many mix code of calculating and drawing. So, if you have better idea, try it and ignore me. I just want lizzie more useful and easier. Get the newest from here, https://github.com/phiming/lizzie/tree/oliver

---

Couldn't agree more,
experienced people can often take less time to re-implement something them-self than to help a contributor to meet their quality standard thought a dept review.

[OlivierBlanvillain]

@phiming Makes sense, I'll try to finish it when I get some more time.

[zsalch]

O, I am still looking forward to seeing this feature...

[zsalch]

@OlivierBlanvillain @phiming
I have tried to add the click feature. Please help to review it.
Source
Pre-release