New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed

phiming wants to merge 0 commits into featurecat:next from phiming:next

phiming commented Sep 30, 2018

#373 use mouse click to navigate in branch window just like in Sabaki

Contributor

zsalch commented Oct 2, 2018

@phiming
Thank you for implementing this great feature.

Please help to check the following before the code review:

Code indentation uses 4 spaces
Variable names are as meaningful as possible
Remove unnecessary blank lines and useless code

more detail can refer to the review comment of @OlivierBlanvillain:
#364 #368

Author

phiming commented Oct 2, 2018

@zsalch
OK, for
1, 3, I will follow it.
2, I will add more comments and modifications to make code more readable.

zsalch reviewed

View reviewed changes

src/main/java/featurecat/lizzie/gui/LizzieFrame.java Outdated

@@ @@ -808,7 +808,7 @@ public void onClicked(int x, int y) { @@
                       // check for board click
                       int[] boardCoordinates = boardRenderer.convertScreenToCoordinates(x, y);
                       int moveNumber = winrateGraph.moveNumber(x, y);

Contributor

zsalch Oct 4, 2018

Please remove the blanks.

src/main/java/featurecat/lizzie/gui/LizzieFrame.java Outdated

@@ @@ -822,6 +822,11 @@ public void onClicked(int x, int y) { @@
                       if (Lizzie.config.showSubBoard && subBoardRenderer.isInside(x, y)) {
                           Lizzie.config.toggleLargeSubBoard();
                       }
+                      if (Lizzie.config.showVariationGraph) {
+                          Lizzie.frame.variationTree.jumpVariationTree(x,y);

Contributor

zsalch Oct 4, 2018

please add a blank after the ','.

src/main/java/featurecat/lizzie/gui/VariationTree.java Outdated

@@ @@ -13,12 +13,15 @@ @@
                   private int XSPACING;
                   private int DOT_DIAM = 11; // Should be odd number
-                  private ArrayList<Integer> laneUsageList;
-                  private BoardHistoryNode curMove;
+                  private ArrayList<Integer> laneUsageList, laneUsageList1;

Contributor

zsalch Oct 4, 2018

The laneUsageList1 should be more sense.

src/main/java/featurecat/lizzie/gui/VariationTree.java Outdated

+                  private ArrayList<Integer> laneUsageList, laneUsageList1;
+                  private BoardHistoryNode curMove, curMove1;
+              	private int posx1, posy1, width1, height1;   //Reserve a copy of varaition window axis

Contributor

zsalch Oct 4, 2018

Please check the indent & variables sense.

src/main/java/featurecat/lizzie/gui/VariationTree.java Outdated

                       YSPACING = (Lizzie.config.showLargeSubBoard() ? 20 : 30);
                       XSPACING = YSPACING;
+              	    posx1=posx;posy1=posy;width1=width;height1=height;  //backup for mouse click event

Contributor

zsalch Oct 4, 2018

Please append spaces on both sides of the '=' and split the assignment statement to new lines.

src/main/java/featurecat/lizzie/gui/VariationTree.java Outdated

                       }
                       drawTree(g, posx + xoffset, curposy, 0, posy + height, node, 0,true);
                   }

Contributor

zsalch Oct 4, 2018

Remove duplicate blank line.

src/main/java/featurecat/lizzie/gui/VariationTree.java Outdated

+                  //Clone from drawTree() method but draw nothing, just caculate (x,y)->BoardNode
+              	public BoardHistoryNode inSubtree(int posx, int posy, int mouseX, int mouseY, int startLane, int maxposy, BoardHistoryNode startNode, int variationNumber)
+              	{

Contributor

zsalch Oct 4, 2018

The '{' should be in the end of line.

Author

phiming commented Oct 4, 2018

Thanks for your careful review.

I use vscode to write and debug those code. It looks good in vscode, but there are still some blanks and spaces I cant find. Is there any tools to check this automatically?

Owner

featurecat commented Oct 4, 2018

I recommend using intellij or eclipse, they have auto formatters

intellij: ctrl alt L

eclipse: ctrl shift f

Owner

featurecat commented Oct 4, 2018

VS code may have its own formatter. I don't know

Contributor

OlivierBlanvillain commented Oct 4, 2018

I'm actually thinking about setting up this mvn plugin to autoformat the code: https://github.com/google/google-java-format. It needs a tiny bit of work to prevent very large lines from becoming unreadable, but once I'm done with that we shouldn't have to review anything related to code format...

OlivierBlanvillain closed this

OlivierBlanvillain force-pushed the next branch from 4fe5f6c to 10875ca Compare

October 6, 2018 22:32

OlivierBlanvillain mentioned this pull request

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

Closed

Contributor

OlivierBlanvillain commented Oct 6, 2018

@phiming I pushed a merge commit to your branch and I think it confused github, it doesn't show your commits here and won't let me reopen int 😕... I opened #391 with your changed plus my merge commit, lets continue from there, sorry about that.

Author

phiming commented Oct 7, 2018

@OlivierBlanvillain
It's ok. Can I help do anything?
I really think it's useful feature especially for windows users who get used of mouse.

Contributor

OlivierBlanvillain commented Oct 7, 2018

I will try to review it ASAP 😄!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet