magefree / magefree/mage

Refactor: getOpponents must be called with second param to ignore leave players in any effects

Open
#13,289 1 comment 0 reactions 0 assignees View on GitHub
refactoring
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.