Refactor: getOpponents must be called with second param to ignore leave players in any effects
- Dominant language
- Java
- Stars
- 2.4k
- Forks
- 940
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 160
Description
from 725c29182b661985bc0f9183a9e427cf865d4edd
Due mtg rules:
* end of turn effects of leave player must keep until end of turn (e.g. include leave players for stats calculations and checks)
* one short and other one time effects and SBA must use actual game info (e.g. exclude leave players);
`Game::getOpponents` already support it, but many effects use default method and include leave players. But it must exclude it (see bugs example with Corrupt ability from 725c29182b661985bc0f9183a9e427cf865d4edd);
Possible solutions:
1. Normal: deprecate one param version of getOpponents and replace it by two params version (~700 effects to check, fix example: 58fbfdd5298371333b4c4b4628c40f6ad8ec3c30);
2. Better: make default getOpponents to exclude leave players (it's 99% of all use cases); but it require some tests. Current code fails in one place only `test_BloodchiefAscension_DieBeforeEndTurn`. So such use cases are low tested. Potentially bugged code to check: regexp search `Opponent.+Condition` -- it must setup additional param to include/exclude leave player.
Bugs example:
* #9028
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with Game::getOpponents and the failing test_BloodchiefAscension_DieBeforeEndTurn. Search for Opponent.+Condition and inspect the effects that call getOpponents without the second parameter, using the linked commits and bug #9028 for context. Done means the chosen getOpponents behavior is applied consistently and the relevant tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- game-dev
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100