Skip to content
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

next: Enhance the variation read and display #412

Merged
merged 5 commits into from
Nov 8, 2018
Merged

next: Enhance the variation read and display #412

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
69fd2a1
Enhance the variation read and display
zsalch Oct 31, 2018
1421734
New move number in the branch
zsalch Nov 3, 2018
9222cd1
Fixed move number when move branch
Nov 4, 2018
4c7a0e3
Fixed the code by review
zsalch Nov 7, 2018
25f2d8d
Merge branch 'next' into next_var_enhance
featurecat Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/featurecat/lizzie/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
public class Config {

public boolean showMoveNumber = false;
public boolean newMoveNubmerInBranch = true;
OlivierBlanvillain marked this conversation as resolved.
Show resolved Hide resolved
public boolean showWinrate = true;
public boolean largeWinrate = false;
public boolean showBlunderBar = true;
Expand Down Expand Up @@ -139,6 +140,7 @@ public Config() throws IOException {
theme = new Theme(uiConfig);

showMoveNumber = uiConfig.getBoolean("show-move-number");
newMoveNubmerInBranch = uiConfig.optBoolean("new-move-number-in-branch", true);
showStatus = uiConfig.getBoolean("show-status");
showBranch = uiConfig.getBoolean("show-leelaz-variation");
showWinrate = uiConfig.getBoolean("show-winrate");
Expand Down
132 changes: 99 additions & 33 deletions src/main/java/featurecat/lizzie/gui/VariationTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class VariationTree {
private int DOT_DIAM = 11; // Should be odd number

private ArrayList<Integer> laneUsageList;
private int laneCount = 0;
private BoardHistoryNode curMove;

public VariationTree() {
Expand All @@ -25,6 +26,7 @@ public void drawTree(
int posy,
int startLane,
int maxposy,
int minposx,
BoardHistoryNode startNode,
int variationNumber,
boolean isMain) {
Expand All @@ -36,17 +38,17 @@ public void drawTree(
int lane = startLane;
// Figures out how far out too the right (which lane) we have to go not to collide with other
// variations
while (lane < laneUsageList.size()
&& laneUsageList.get(lane) <= startNode.getData().moveNumber + depth) {
int moveNumber = startNode.getData().moveNumber;
while (lane < laneUsageList.size() && laneUsageList.get(lane) <= moveNumber + depth) {
// laneUsageList keeps a list of how far down it is to a variation in the different "lanes"
laneUsageList.set(lane, startNode.getData().moveNumber - 1);
laneUsageList.set(lane, moveNumber - 1);
lane++;
}
if (lane >= laneUsageList.size()) {
laneUsageList.add(0);
}
if (variationNumber > 1) laneUsageList.set(lane - 1, startNode.getData().moveNumber - 1);
laneUsageList.set(lane, startNode.getData().moveNumber);
if (variationNumber > 1) laneUsageList.set(lane - 1, moveNumber - 1);
laneUsageList.set(lane, moveNumber);

// At this point, lane contains the lane we should use (the main branch is in lane 0)

Expand All @@ -58,60 +60,74 @@ public void drawTree(
if (lane > 0) {
if (lane - startLane > 0 || variationNumber > 1) {
// Need a horizontal and an angled line
g.drawLine(
drawLine(
g,
curposx + dotoffset,
posy + dotoffset,
curposx + dotoffset - XSPACING,
posy + dotoffset - YSPACING);
g.drawLine(
posy + dotoffset - YSPACING,
minposx);
drawLine(
g,
posx + (startLane - variationNumber) * XSPACING + 2 * dotoffset,
posy - YSPACING + dotoffset,
curposx + dotoffset - XSPACING,
posy + dotoffset - YSPACING);
posy + dotoffset - YSPACING,
minposx);
} else {
// Just an angled line
g.drawLine(
drawLine(
g,
curposx + dotoffset,
posy + dotoffset,
curposx + 2 * dotoffset - XSPACING,
posy + 2 * dotoffset - YSPACING);
posy + 2 * dotoffset - YSPACING,
minposx);
}
}

// Draw all the nodes and lines in this lane (not variations)
Color curcolor = g.getColor();
if (startNode == curMove) {
g.setColor(Color.green.brighter().brighter());
}
if (startNode.previous().isPresent()) {
g.fillOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawOval(curposx, posy, DOT_DIAM, DOT_DIAM);
} else {
g.fillRect(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawRect(curposx, posy, DOT_DIAM, DOT_DIAM);
if (curposx > minposx && posy > 0) {
OlivierBlanvillain marked this conversation as resolved.
Show resolved Hide resolved
if (startNode == curMove) {
g.setColor(Color.green.brighter().brighter());
}
if (startNode.previous().isPresent()) {
g.fillOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawOval(curposx, posy, DOT_DIAM, DOT_DIAM);
} else {
g.fillRect(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawRect(curposx, posy, DOT_DIAM, DOT_DIAM);
}
}
g.setColor(curcolor);

// Draw main line
while (cur.next().isPresent() && posy + YSPACING < maxposy) {
posy += YSPACING;
cur = cur.next().get();
if (cur == curMove) {
g.setColor(Color.green.brighter().brighter());
if (curposx > minposx && posy > 0) {
if (cur == curMove) {
g.setColor(Color.green.brighter().brighter());
}
g.fillOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(curcolor);
g.drawLine(
curposx + dotoffset,
posy - 1,
curposx + dotoffset,
posy - YSPACING + 2 * dotoffset + 2);
}
g.fillOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(Color.BLACK);
g.drawOval(curposx, posy, DOT_DIAM, DOT_DIAM);
g.setColor(curcolor);
g.drawLine(
curposx + dotoffset, posy - 1, curposx + dotoffset, posy - YSPACING + 2 * dotoffset + 2);
}
// Now we have drawn all the nodes in this variation, and has reached the bottom of this
// variation
// Move back up, and for each, draw any variations we find
while (cur.previous().isPresent() && cur != startNode) {
BoardHistoryNode end = isMain ? null : startNode;
while (cur.previous().isPresent() && cur != end) {
cur = cur.previous().get();
int curwidth = lane;
// Draw each variation, uses recursion
Expand All @@ -121,7 +137,7 @@ public void drawTree(
// every variation that has a variation (sort of))
Optional<BoardHistoryNode> variation = cur.getVariation(i);
if (variation.isPresent()) {
drawTree(g, posx, posy, curwidth, maxposy, variation.get(), i, false);
drawTree(g, posx, posy, curwidth, maxposy, minposx, variation.get(), i, false);
}
}
posy -= YSPACING;
Expand Down Expand Up @@ -166,6 +182,56 @@ public void draw(Graphics2D g, int posx, int posy, int width, int height) {
node = node.previous().get();
curposy -= YSPACING;
}
drawTree(g, posx + xoffset, curposy, 0, posy + height, node, 0, true);
laneCount = 0;
OlivierBlanvillain marked this conversation as resolved.
Show resolved Hide resolved
int lane = getCurLane(node, curMove, curposy, posy + height, true);
int startx = posx + xoffset;
if (((lane + 1) * XSPACING + xoffset + DOT_DIAM + strokeRadius - width) > 0) {
startx = startx - ((lane + 1) * XSPACING + xoffset + DOT_DIAM + strokeRadius - width);
}
drawTree(g, startx, curposy, 0, posy + height, posx + strokeRadius, node, 0, true);
}

private void drawLine(Graphics g, int x1, int y1, int x2, int y2, int minx) {
if (x1 <= minx && x2 <= minx) {
return;
}
int nx1 = x1, ny1 = y1, nx2 = x2, ny2 = y2;
if (x1 > minx && x2 <= minx) {
ny2 = y2 - (x1 - minx) / (x1 - x2) * (y2 - y1);
nx2 = minx;
} else if (x2 > minx && x1 <= minx) {
ny1 = y1 - (x2 - minx) / (x2 - x1) * (y1 - y2);
nx1 = minx;
}
g.drawLine(nx1, ny1, nx2, ny2);
}

private int getCurLane(
BoardHistoryNode start, BoardHistoryNode curMove, int curposy, int maxy, boolean isMain) {
BoardHistoryNode next = start;
int nexty = curposy;
while (next.next().isPresent() && nexty + YSPACING < maxy) {
nexty += YSPACING;
next = next.next().get();
}
BoardHistoryNode end = isMain ? null : start;
while (next.previous().isPresent() && next != end) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be clearer if you inline that condition as follows:

&& (isMain || next != start)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

next = next.previous().get();
for (int i = 1; i < next.numberOfChildren(); i++) {
laneCount++;
if (next.findIndexOfNode(curMove, true) == i) {
return laneCount;
}
Optional<BoardHistoryNode> variation = next.getVariation(i);
if (variation.isPresent()) {
int subLane = getCurLane(variation.get(), curMove, nexty, maxy, false);
if (subLane > 0) {
return subLane;
}
}
}
nexty -= YSPACING;
}
return 0;
}
}
34 changes: 26 additions & 8 deletions src/main/java/featurecat/lizzie/rules/Board.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public void comment(String comment) {
public void moveNumber(int moveNumber) {
synchronized (this) {
BoardData data = history.getData();
data.moveNumber = moveNumber;
if (data.lastMove.isPresent()) {
int[] moveNumberList = history.getMoveNumberList();
moveNumberList[Board.getIndex(data.lastMove.get()[0], data.lastMove.get()[1])] = moveNumber;
Expand Down Expand Up @@ -205,7 +204,6 @@ public void removeStone(int x, int y, Stone color) {
Stone oriColor = stones[getIndex(x, y)];
stones[getIndex(x, y)] = Stone.EMPTY;
zobrist.toggleStone(x, y, oriColor);
data.moveNumber = 0;
data.moveNumberList[Board.getIndex(x, y)] = 0;

Lizzie.frame.repaint();
Expand Down Expand Up @@ -241,10 +239,20 @@ public void addNodeProperties(Map<String, String> properties) {
* @param color the type of pass
*/
public void pass(Stone color) {
pass(color, false, false);
}

/**
* The pass. Thread safe
*
* @param color the type of pass
* @param newBranch add a new branch
*/
public void pass(Stone color, boolean newBranch, boolean dummy) {
synchronized (this) {

// check to see if this move is being replayed in history
if (history.getNext().map(n -> !n.lastMove.isPresent()).orElse(false)) {
if (history.getNext().map(n -> !n.lastMove.isPresent()).orElse(false) && !newBranch) {
// this is the next move in history. Just increment history so that we don't erase the
// redo's
history.next();
Expand All @@ -258,7 +266,10 @@ public void pass(Stone color) {
Stone[] stones = history.getStones().clone();
Zobrist zobrist = history.getZobrist();
int moveNumber = history.getMoveNumber() + 1;
int[] moveNumberList = history.getMoveNumberList().clone();
int[] moveNumberList =
newBranch && history.getNext().isPresent()
? new int[Board.boardSize * Board.boardSize]
: history.getMoveNumberList().clone();

// build the new game state
BoardData newState =
Expand All @@ -274,14 +285,15 @@ public void pass(Stone color) {
history.getData().whiteCaptures,
0,
0);
newState.dummy = dummy;

// update leelaz with pass
Lizzie.leelaz.playMove(color, "pass");
if (Lizzie.frame.isPlayingAgainstLeelaz)
Lizzie.leelaz.genmove((history.isBlacksTurn() ? "W" : "B"));

// update history with pass
history.addOrGoto(newState);
history.addOrGoto(newState, newBranch);

Lizzie.frame.repaint();
}
Expand Down Expand Up @@ -335,7 +347,7 @@ public void place(int x, int y, Stone color, boolean newBranch) {

// check to see if this coordinate is being replayed in history
Optional<int[]> nextLast = history.getNext().flatMap(n -> n.lastMove);
if (nextLast.isPresent() && nextLast.get()[0] == x && nextLast.get()[1] == y) {
if (nextLast.isPresent() && nextLast.get()[0] == x && nextLast.get()[1] == y && !newBranch) {
// this is the next coordinate in history. Just increment history so that we don't erase the
// redo's
history.next();
Expand All @@ -355,9 +367,14 @@ public void place(int x, int y, Stone color, boolean newBranch) {
Zobrist zobrist = history.getZobrist();
Optional<int[]> lastMove = Optional.of(new int[] {x, y});
int moveNumber = history.getMoveNumber() + 1;
int[] moveNumberList = history.getMoveNumberList().clone();
int moveMNNumber =
history.getMoveMNNumber() > -1 && !newBranch ? history.getMoveMNNumber() + 1 : -1;
int[] moveNumberList =
newBranch && history.getNext().isPresent()
? new int[Board.boardSize * Board.boardSize]
: history.getMoveNumberList().clone();

moveNumberList[Board.getIndex(x, y)] = moveNumber;
moveNumberList[Board.getIndex(x, y)] = moveMNNumber > -1 ? moveMNNumber : moveNumber;

// set the stone at (x, y) to color
stones[getIndex(x, y)] = color;
Expand Down Expand Up @@ -396,6 +413,7 @@ public void place(int x, int y, Stone color, boolean newBranch) {
wc,
nextWinrate,
0);
newState.moveMNNumber = moveMNNumber;

// don't make this coordinate if it is suicidal or violates superko
if (isSuicidal > 0 || history.violatesSuperko(newState)) return;
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/featurecat/lizzie/rules/BoardData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

public class BoardData {
public int moveNumber;
public int moveMNNumber;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to store 2 move numbers? Can't we merge these two into one field?

Copy link
Contributor Author

@zsalch zsalch Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

In current code, the moveNumber actual keeps the BoardData sequence number.
Many logic depend on the moveNumber.
The moveMNNumber for display move number, such as 'MN' attribute, move number in the branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really happy with keeping all this state around, its quite hard to follow to be honest. Do you think we could we instead recompute moveNumber on the fly? I also think the naming doesn't help here. Maybe we should rename these things as follows:

  • moveNumber → depth
  • moveMNNumber → numberlabel

WDYT?

public Optional<int[]> lastMove;
public int[] moveNumberList;
public boolean blackToPlay;
public boolean dummy;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does dummy mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some sgf file contains the AB/AW/AE attributes. For these attributes, it is better to create a dummy BoardData.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still, I don't understand. Why do we need dummy BoardData?


public Stone lastMoveColor;
public Stone[] stones;
Expand Down Expand Up @@ -37,10 +39,12 @@ public BoardData(
int whiteCaptures,
double winrate,
int playouts) {
this.moveMNNumber = -1;
this.moveNumber = moveNumber;
this.lastMove = lastMove;
this.moveNumberList = moveNumberList;
this.blackToPlay = blackToPlay;
this.dummy = false;

this.lastMoveColor = lastMoveColor;
this.stones = stones;
Expand Down Expand Up @@ -72,6 +76,11 @@ public static BoardData empty(int size) {
*/
public void addProperty(String key, String value) {
SGFParser.addProperty(properties, key, value);
if ("N".equals(key) && comment.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like to have SGF parsing spreading all over the code base, it seams that this code doesn't belong to the BoardData class but should be moved to SGFParser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think these code needs to be refactored and optimized in the next version.

comment = value;
} else if ("MN".equals(key)) {
moveMNNumber = Integer.parseInt(getOrDefault("MN", "-1"));
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/featurecat/lizzie/rules/BoardHistoryList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ public int getMoveNumber() {
return head.getData().moveNumber;
}

public int getMoveMNNumber() {
return head.getData().moveMNNumber;
}

public int[] getMoveNumberList() {
return head.getData().moveNumberList;
}
Expand Down
Loading