Skip to content

Fix Simulator for west-round terminal states #196

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

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

Fix Simulator for west-round terminal states #196

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ad3bb0
fix: check terminal case
tsubakisakura Feb 18, 2026
f103cf1
fix: fix all last condition
tsubakisakura Feb 18, 2026
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
13 changes: 9 additions & 4 deletions python/mjai/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ def get_next_state(self, jsonl_path: Path | str) -> dict | None:
last_kyoku_riichi_count += 1

if first_start_kyoku_event is None:
# There are terminal cases where Match returns only
# start_game/end_game without any kyoku in continue mode.
# Treat them as a clean finish, not a runtime error.
event_types = {event["type"] for event in events}
if event_types.issubset({"start_game", "end_game"}):
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

event_types.issubset({"start_game", "end_game"}) is also True for an incomplete log that contains only start_game (or even an empty events list), which would be treated as a clean finish and could mask real failures. Consider requiring that end_game is actually present (e.g., check that event_types == {"start_game","end_game"} or at least that both types exist) before returning None.

Suggested change
if event_types.issubset({"start_game", "end_game"}):
if event_types.issubset({"start_game", "end_game"}) and "end_game" in event_types:

Copilot uses AI. Check for mistakes.
Copy link
Owner

@smly smly Feb 20, 2026

Choose a reason for hiding this comment

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

指摘ありがとうございます！get_next_state() が呼ばれる条件を精査します。

return None
raise RuntimeError("can't start game")
scores: list[int] = first_start_kyoku_event["scores"] # type: ignore
last_delta: list[int] = [0, 0, 0, 0]
Expand All @@ -155,7 +161,9 @@ def get_next_state(self, jsonl_path: Path | str) -> dict | None:
honba: int = last_start_kyoku_event["honba"] # type: ignore
kyotaku: int = last_start_kyoku_event["kyotaku"] # type: ignore
oya: int = last_start_kyoku_event["oya"] # type: ignore
if bakaze != "E" and kyoku >= 4:
# 南1局 → `4` のようにゼロ基準の値に変換する
zero_indexed_kyoku = kyoku_to_zero_indexed_kyoku(bakaze, kyoku)
if zero_indexed_kyoku >= 7:
Comment on lines +164 to +166
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

This changes the terminal-state logic to use zero_indexed_kyoku >= 7 (covering West-round hands too), but there doesn’t appear to be a regression test that exercises a West-round continuation (e.g., W1/W2) and asserts get_next_state() returns None when it should. Adding a focused unit/integration test in tests/test_game_simulator.py for a West-round terminal scenario (and the new start_game/end_game-only case) would help prevent future regressions.

Copilot uses AI. Check for mistakes.
Copy link
Owner

@smly smly Feb 20, 2026

Choose a reason for hiding this comment

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

@tsubakisakura これは恥ずかしい見落とし... 修正ありがとうございます！Copilot のコメントしている regression test は不要です。

# オーラス
# - 親の delta が positive かつ 1位なら終了 (True)
# - 親の delta が 0 以下なら終了 (True)
Expand All @@ -164,9 +172,6 @@ def get_next_state(self, jsonl_path: Path | str) -> dict | None:
if last_delta[oya] <= 0:
return None

# 南1局 → `4` のようにゼロ基準の値に変換する
zero_indexed_kyoku = kyoku_to_zero_indexed_kyoku(bakaze, kyoku)

# 親の連続か否か
if last_delta[oya] > 0:
return {
Expand Down
Loading