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.

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

Avoid use recursion #900

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

yzyray
Copy link
Contributor

@yzyray yzyray commented Sep 13, 2021

Recursion is slow and lead to StackOverflowError when saving extremly big analyed game like:
test52_52.zip

@kaorahi
Copy link
Contributor

kaorahi commented Sep 20, 2021

It seems that ( and ) are missing for variations. (;B[aa](;W[ba])(;W[ab])) is saved as (;B[aa];W[ba];W[ab]) wrongly.

  1. Paste the first SGF to Lizzie.
  2. Type Ctrl-C.
  3. Type Ctrl-V.

Then the variation tree is changed with this PR. It is not changed in the original Lizzie 0.7.4.

@kaorahi
Copy link
Contributor

kaorahi commented Sep 20, 2021

a crude fix?

--- a/src/main/java/featurecat/lizzie/rules/SGFParser.java
+++ b/src/main/java/featurecat/lizzie/rules/SGFParser.java
@@ -684,14 +684,28 @@ public class SGFParser {
     // *  with 'xy' = coordinates ; or 'tt' for pass.
 
     // Write variation tree
+    BoardHistoryNode markerBeg = new BoardHistoryNode(null);
+    BoardHistoryNode markerEnd = new BoardHistoryNode(null);
     Stack<BoardHistoryNode> stack = new Stack<>();
     stack.push(history.getCurrentHistoryNode());
     while (!stack.isEmpty()) {
       BoardHistoryNode cur = stack.pop();
+      if (cur == markerBeg) {
+        builder.append('(');
+        continue;
+      }
+      if (cur == markerEnd) {
+        builder.append(')');
+        continue;
+      }
       builder.append(generateNode(board, cur));
+      boolean hasBrothers = (cur.numberOfChildren() > 1);
       if (cur.numberOfChildren() >= 1) {
-        for (int i = cur.numberOfChildren() - 1; i >= 0; i--)
+        for (int i = cur.numberOfChildren() - 1; i >= 0; i--) {
+          if (hasBrothers) stack.push(markerEnd);
           stack.push(cur.getVariations().get(i));
+          if (hasBrothers) stack.push(markerBeg);
+        }
       }
     }
     // close file

@yzyray
Copy link
Contributor Author

yzyray commented Sep 21, 2021

Yes, I forgot the important "()",I think your fix is great.

kaorahi pushed a commit to kaorahi/lizzie that referenced this pull request Oct 26, 2021
@kaorahi kaorahi mentioned this pull request Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants