fix: Use kan tile as winning tile for chankan ron in end_kyoku modal - #165
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes winning-tile inference for chankan ron in the UI’s end_kyoku result enrichment, so the end-of-hand modal shows the kan tile (kakan/ankan) instead of incorrectly falling back to the previous discard.
Changes:
- Extend ron winning-tile inference to also consider
kakanandankanevents from the target (chankan). - Add a fallback to
consumed[0]whenankanevents lackpai.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should not mark found when neither pai nor consumed is available', () => { | ||
| const events: MjaiEvent[] = [ | ||
| makeStartKyoku(), | ||
| { type: 'tsumo', actor: 0, pai: '1s' }, | ||
| { type: 'dahai', actor: 0, pai: '1m' }, | ||
| // Malformed hora with no pai and no prior matching event | ||
| { type: 'hora', actor: 1, target: 0, deltas: [0, 8000, -8000, 0], ura_markers: [] }, | ||
| { type: 'end_kyoku' }, | ||
| ]; | ||
| const gs = new GameState(events); | ||
| gs.jumpTo(events.length); | ||
| const state = gs.getState(); | ||
| const results = state.lastEvent?.meta?.results; | ||
| expect(results).toBeDefined(); | ||
| // Should find the dahai from player 0 (target) | ||
| expect(results[0].winningTile).toBe('1m'); | ||
| }); |
There was a problem hiding this comment.
This test name says it verifies the “neither pai nor consumed is available” case, but the event sequence doesn’t include a kakan/ankan without pai/consumed; it asserts the normal ron inference from a dahai. Renaming the test (or adjusting the events + assertions to actually cover the missing-tile case, e.g. via a spy on console.warn) would make the intent clear.
There was a problem hiding this comment.
Agreed. Renamed the test to "should infer winning tile from dahai for normal ron" to accurately reflect what it covers.
dahaievents from the target player, so for chankan ron the previous discard was incorrectly shown as the winning tilekakanandankanevents (with fallback toconsumed[0]forankanwhich has nopaifield)Thanks for the bug report! @Apricot-S