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

annotations: Crash on invalid move annotations #113

Closed
junghon3709 opened this issue Jun 11, 2023 · 7 comments · Fixed by #123
Closed

annotations: Crash on invalid move annotations #113

junghon3709 opened this issue Jun 11, 2023 · 7 comments · Fixed by #123

Comments

@junghon3709
Copy link

I have the following annofen file:

v1 r1bakab1r/4c4/4c4/p1[n<a+1-2>]3p1p/9/3CC1P2/P1P1P3P/2N3N2/4A4/2BAK1B1R w - - 0 12

This renders successfully.

However, should I change the numbers in the arrows to:

v1 r1bakab1r/4c4/4c4/p1[n<a+1-3>]3p1p/9/3CC1P2/P1P1P3P/2N3N2/4A4/2BAK1B1R w - - 0 12,

an error message pops up:

Traceback (most recent call last):
  File "`PATH`", line 8, in <module>
    sys.exit(main())
  File "`PATH`/__main__.py", line 260, in main
    run(options)
  File "`PATH`/__main__.py", line 60, in run
    atoms_to_put = list(iterate_annofen_tokens(content))
  File "/home/~/Documents/Projects/Xiangqi-Analyser/venv/lib/python3.8/site-packages/xiangqi_setup/file_formats/annofen.py", line 91, in iterate_annofen_tokens
    raise ValueError(f'Malformed annoFEN token: {character!r}')
ValueError: Malformed annoFEN token: '['
@hartwork
Copy link
Owner

hartwork commented Jun 11, 2023

Hi @junghon3709 nice to meet you!

I confirm the issue, here's what I did to reproduce based on your data on the terminal:

Click here to expand for details
# cat issue-113.py 
from xiangqi_setup.file_formats.annofen import iterate_annofen_tokens, _escaped_annotation_names

with open('issue-113.annofen') as f:
   list(iterate_annofen_tokens(f.read()))

# cat issue-113.annofen 
v1 r1bakab1r/4c4/4c4/p1[n<a+1-3>]3p1p/9/3CC1P2/P1P1P3P/2N3N2/4A4/2BAK1B1R w - - 0 12

# python3.10 issue-113.py 
Traceback (most recent call last):
  File "[..]/xiangqi-setup/issue-113.py", line 4, in <module>
    list(iterate_annofen_tokens(f.read()))
  File "[..]/xiangqi-setup/xiangqi_setup/file_formats/annofen.py", line 92, in iterate_annofen_tokens
    raise ValueError(f'Malformed annoFEN token: {character!r}')
ValueError: Malformed annoFEN token: '['

What is happening here is that "+1-3" is not a valid move to the eyes of xiangqi-setup and the way the parser works makes it end up with this admittedly unhelpful-to-humans error. For each supported move there is one SVG file, e.g. see this directory. If a move is legal to any of the pieces, it is supported, e.g. "a+1-2" is one out of eight for the knights/horses. The full list is this:

+0+1 +0-1 +0+2 +0-2 +0+3 +0-3 +0+4 +0-4 +0+5 +0-5 +0+6 +0-6 +0+7 +0-7 +0+8 +0-8 +0+9 +0-9 +1+0 -1+0 +1+1 +1-1 -1+1 -1-1 +1+2 +1-2 -1+2 -1-2 +2+0 -2+0 +2+1 +2-1 -2+1 -2-1 +2+2 +2-2 -2+2 -2-2 +3+0 -3+0 +4+0 -4+0 +5+0 -5+0 +6+0 -6+0 +7+0 -7+0 +8+0 -8+0

I have some hope that "+1-3" was in part a typo and that legal moves are enough for your use case? Then I would only need to improve the error message. I have some idea how to do that. What do you think, and what was your intention when trying "+1-3"?

Best, Sebastian

@hartwork
Copy link
Owner

@junghon3709 what do you think?

1 similar comment
@hartwork
Copy link
Owner

hartwork commented Jul 7, 2023

@junghon3709 what do you think?

@junghon3709
Copy link
Author

I see now, I guess it makes sense to change the error message

@hartwork hartwork changed the title Some numbers work with annofen while some don't annotations: Crash on invalid move annotations Jul 24, 2023
@hartwork
Copy link
Owner

With #123 merged, the output would say…

xiangqi_setup.annotations.InvalidAnnotationCode: Invalid annotation code 'a+1-3'

…now.

@hartwork
Copy link
Owner

Version 2.2.0 with #123 merged released to GitHub and PyPI just now.

@junghon3709
Copy link
Author

Thank you for the change!

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 a pull request may close this issue.

2 participants