[MKM] Implement Expedited Inheritance #11791
Conversation
| @@ -62,6 +62,8 @@ A Characteristic Defining Ability (CDA) is an ability that defines a characteris | |||
| // set to the turn number on your next turn. | |||
| private int effectStartingStepNum = 0; // Some continuous are waiting for the next step of a kind. | |||
| // Avoid miscancelling if the start step is of that kind. | |||
| private boolean useCustomController = false; | |||
| private UUID customControllerId; | |||
There was a problem hiding this comment.
Smells bad. Do not modify game engine without reason/tests.
1
"Make card playable" code uses a defined playerId, so if you call makeCardPlayable with custom playerId then that playerId will be able to cast it, not source/ability controller:
So rewrite your code without game engine changes or make details examples and a detailed explanation (what you want to do, what you get without modification, what you get with modification).
2
method uses the ability's controller to calculate the duration
Use full method version instead -- it allows to customize duration and player:
There was a problem hiding this comment.
I'm not sure if it's possible to implement this card fully without making a change to the game engine. The issue is that the code in ContinuousEffectImpl.java that calculates the duration for the effect always uses the controllerId of the effect's source:
This works fine for similar cards that allow other players to exile cards from the top of their libraries and play them for a certain duration, such as [[Rocco, Street Chef]] because in Rocco's case the duration is still dependent on the effect's controller (Until your next end step, each player may play the card they exiled this way).
With [[Expedited Inheritance]], the duration during which the card can be played depends not on the effect's controller, but the controller/owner of the exiled cards (They may play those cards until the end of their next turn).
I solved this by changing the setStartingControllerAndTurnNum call in ContinuousEffectImpl.java to conditionally use a dynamic startingController instead of always defaulting to source.getControllerId(). I'm curious if you have any thoughts on a better approach for this, or if it's possible to do without altering game engine code.
Either way, this is the only place where that new Id is used; the existing logic that determines which player is able to play the card still works are expected and is unchanged.
There was a problem hiding this comment.
What about creating your own effect that extends AsThoughEffectImpl with a custom duration and overrides isInactive? Then you could do the check yourself with the correct player ID. See EscapeToTheWildsMayPlayEffect, for an example.
There was a problem hiding this comment.
Thanks for the tip, I've made those changes.
I'm guessing the build test failure is unrelated to these changes as it seems to be affecting other PRs as well?
There was a problem hiding this comment.
Yeah, don't worry about the travis build, I'll go through PRs once the test failure on master is addressed
| @Override | ||
| public boolean applies(UUID sourceId, Ability source, UUID affectedControllerId, Game game) { | ||
| UUID objectIdToCast = CardUtil.getMainCardId(game, sourceId); | ||
| return cardOwnerId == affectedControllerId |
There was a problem hiding this comment.
compare UUID with equals
| UUID playerId = creature.getControllerId(); | ||
| Player player = game.getPlayer(playerId); | ||
| String message = impulseDrawAmount > 1 ? | ||
| "Exile " + impulseDrawAmount + " cards from the top of your library. Until the end of your next turn, you may play those cards." |
There was a problem hiding this comment.
(can use CardUtil.numberToText for message, not required)
#11516