Skip to content

Fix Simulator for west-round terminal states#196

Open
tsubakisakura wants to merge 2 commits intosmly:mainfrom
tsubakisakura:fix-terminal-case
Open

Fix Simulator for west-round terminal states#196
tsubakisakura wants to merge 2 commits intosmly:mainfrom
tsubakisakura:fix-terminal-case

Conversation

@tsubakisakura
Copy link

Summary

  • Simulator.get_next_state() の終局判定を zero_indexed_kyoku ベースに修正
  • W2 など東場以外での最終盤面を正しく終局扱いできるようにした
  • start_game/end_game のみ(start_kyoku なし)の continue 結果を正常終了として扱うガードを追加

Why

  • seed=(10030,25030) で run_0 終了後に不要な run_1 が走り、RuntimeError: can't start game が発生していた
  • 原因は Python 側の終局判定条件が不十分で、終局済みでも continue し得ること

再現コード

#!/usr/bin/env python3
import mjai

submissions = [
    "examples/rulebase.zip",
    "examples/rulebase.zip",
    "examples/rulebase.zip",
    "examples/rulebase.zip",
]

seed = (10030, 25030)
mjai.Simulator(submissions, logs_dir="./logs", seed=seed).run()

@smly smly requested review from Copilot and smly and removed request for smly February 20, 2026 10:15
@smly smly self-assigned this Feb 20, 2026
@smly smly added the bug Something isn't working label Feb 20, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Simulator.get_next_state() terminal-state detection in continue mode by basing the “final hand and later” check on a zero-indexed kyoku value, ensuring West-round states (e.g., W2) are handled as terminal when appropriate, and adds a guard to treat start_game/end_game-only logs as a clean finish.

Changes:

  • Add a guard to return None (clean finish) when a continue-mode log contains only start_game/end_game events.
  • Convert (bakaze, kyoku) to a unified zero_indexed_kyoku earlier and use it for terminal (“オーラス以降”) detection.
  • Remove redundant late conversion by reusing zero_indexed_kyoku for subsequent next-state computation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# 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

Choose a reason for hiding this comment

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

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

Comment on lines +164 to +166
# 南1局 → `4` のようにゼロ基準の値に変換する
zero_indexed_kyoku = kyoku_to_zero_indexed_kyoku(bakaze, kyoku)
if zero_indexed_kyoku >= 7:
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

Choose a reason for hiding this comment

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants