Add Xiangqi (Chinese Chess) implementation to OpenSpiel#1512
Add Xiangqi (Chinese Chess) implementation to OpenSpiel#1512Arahan-kujur wants to merge 6 commits intogoogle-deepmind:masterfrom
Conversation
|
Thanks! |
lanctot
left a comment
There was a problem hiding this comment.
Can you add the game to docs/games.md ?
lanctot
left a comment
There was a problem hiding this comment.
Can you:
- Add the game to
pyspiel_test.py - Add a playthrough
See steps 5 & 11 of the "Adding a Game" guidelines here: https://github.com/google-deepmind/open_spiel/blob/master/docs/developer_guide.md#adding-a-game
…integration tests Made-with: Cursor
Added Xiangqi to |
Great, thanks. Did you see this comment? #1512 (comment) |
Yes, thanks — I’ve addressed that comment in the latest commit. |
|
Something seems wrong with the usual tests, but the wheels tests passed. So it's probably fine. Can you make a one-line change (maybe add a comment somewhere) and push it so it can trigger a new invocation of the tests? |
I’ve pushed a small change to trigger a new test run. |
|
Ok, I ran into the same issue again, and each time I could not see a log. So this time I'm watching it. https://github.com/google-deepmind/open_spiel/actions/runs/23845020407/job/69518259486 If it happens again, I suspect one of the simulation tests might be stuck in an infinite loop. Especially if it's a game where uniform random policy could go on forever, then it's likely just getting stuck in an endless game. (To fix these cases, we usually put a limit on the number of moves and call it a draw after that number of moves have been applied without a winner. You'd have to add that to the implementation.) |
|
Hypothesis confirmed. This is the last part of the output: |
|
Ok so the fix is to keep a variable in You don't need to make the maximum a parameter to the game; you can just put a sensible constant at the top of the header file. But then you also need to return that number in |
|
|
Thanks for the clarification. I currently added the move limit check using the existing |
|
Nope, no need -- this is perfrect. |
Great, thanks! Is everything good to merge now, or is there anything else you'd like me to adjust? |
I only took a quick look, but it seems mostly fine. I'm currently on vacation, so I'll import it when I get back to work (after April 14th). There may be 1-2 more things to do if there's something major in the import, but that's rare. So expect it to be merged mid to late April. |
Thanks for taking a look, and enjoy your vacation! Happy to make any adjustments if something comes up during the import. |
This PR adds an implementation of Xiangqi (Chinese Chess) to OpenSpiel.
Features:
Includes unit tests for move generation, rule enforcement, and state transitions.