Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clicking on "face-down creature" link on log/stack shows actual card, should show morph card #567

Closed
melvinzhang opened this issue Dec 17, 2015 · 19 comments
Assignees
Labels
Milestone

Comments

@melvinzhang
Copy link
Contributor

kannikkiy @ http://www.slightlymagic.net/forum/viewtopic.php?f=82&t=17929&p=190003#p189990 reports the following bug:

AI casts Ainok Survivalist with morph, it goes onto the stack as "Put [face-down creature] onto the battlefield". The part in [] is a blue link, clicking on the link shows the Ainok Survivalist card in the center of the battlefield. It should show the Morph reminder card.

@melvinzhang melvinzhang added this to the 1.69 milestone Dec 17, 2015
@melvinzhang
Copy link
Contributor Author

Bug also occurs when clicking link in the log.

@melvinzhang melvinzhang removed the UI label Dec 17, 2015
@melvinzhang
Copy link
Contributor Author

UI gets cardId as input, MagicCard does not know whether it was cast as morph or using normal method.

@melvinzhang
Copy link
Contributor Author

one solution is for getMagicCard to return the proper context, for spell on stack, it should return MagicCardOnStack instead of MagicCard, for permanent on the battlefield it should return MagicPermanent. Subsequent methods should use only the interface MagicObject instead of MagicCard.

@melvinzhang melvinzhang added the UI label Dec 17, 2015
@melvinzhang melvinzhang changed the title clicking on "face-down creature" link on the stack shows actual card, should show morph card clicking on "face-down creature" link on log/stack shows actual card, should show morph card Dec 17, 2015
@melvinzhang
Copy link
Contributor Author

doesn't quite work, all the downstream methods require MagicCard, as it looks for the location

@melvinzhang
Copy link
Contributor Author

@lodici any thoughts on how we can approach this bug?

@melvinzhang
Copy link
Contributor Author

Quick summary of the issue: the problem is because MagicCard has no knowledge of morph, it is always the actual card. Morph creates a special MagicCardOnStack that returns the morph definition as getCardDefinition, so that when you mouse over the source in the stack, it shows the Morph reminder card and not the actual card. However, clicking on the link in the stack/log uses the MagicCard which shows the true card.

@lodici
Copy link
Member

lodici commented Dec 18, 2015

I think we will probably need to update GameViewerInfo.getMagicCard() to return not a MagicCard but a viewer object containing the required info for displaying the correct image. It actually loses info by only returning aMagicCard object since getMagicCard() can tell whether the card Id is a permanent or on the stack.

@melvinzhang
Copy link
Contributor Author

Sounds good! A custom viewer object makes a lot of sense, I was thinking along the lines of modifying the API for existing game objects but a custom viewer object would be much better. Can I trouble you to design the class for the viewer as you are more familiar with the kind of data needed for showing the card properly?

@lodici lodici self-assigned this Dec 18, 2015
@lodici
Copy link
Member

lodici commented Dec 18, 2015

The same problem occurs when running the play from hand animation in that it shows you the front face so I will look at that as well since it is related.

@melvinzhang
Copy link
Contributor Author

Good catch on the animation bug. Thanks for taking care of this.

@lodici
Copy link
Member

lodici commented Dec 19, 2015

I have solved the log entry but have hit a bit of brick wall when it comes to the AI-plays-from-hand animation and possibly the stack item. In a nutshell I have identified the MagicCard being played (Ainok Survivalist) but do know how to determine whether it is being played face down or not. @melvinzhang is there a property/method I can use and if not can one be added?

In the case of the animation, the card has left the hand but has yet to appear in the stack.

@melvinzhang
Copy link
Contributor Author

MagicCard doesn't know, you need to look at the MagicCardOnStack. Similarly for a Morph permanent on the battlefield, you have to check with the MagicPermanent, the underlying MagicCard is no different from a non-morph permanent.

From the model point of view, the resolution of the Morph cast event removes the card and adds it to the stack in one single event. If I'm reading the code right, after the event occurs, then the animation plays as updateGameView is after executeNextEventOrPhase. So in that case the card is already on the stack, if the playing of the card leads to a item on the stack, it is better to show the card definition obtained from MagicCardOnStack instead of showing the MagicCard.

@melvinzhang
Copy link
Contributor Author

I briefly looked over the getPlayCardAnimationInfo method, one way to get the card on stack is to do a "diff" of the stack between the oldinfo and newinfo. The extra stack item will be the card on stack for the just played card.

@lodici
Copy link
Member

lodici commented Dec 19, 2015

Awesome, so if I just get newGameInfo.getStack().get(0).itemOnStack.getCardDefinition() it will always be the card definition visible to the player? Which in this case will be morph.

@melvinzhang
Copy link
Contributor Author

Yes, if there is a new card on stack, we should use its getCardDefinition instead of the one from MagicCard. Note that there may not always be a new card on stack, some cards do not use the stack, eg lands.

@lodici
Copy link
Member

lodici commented Dec 19, 2015

That should be ok, I can test for this. I have never understood why there are all these different "card" classes - in my mind you should only need MagicCardDefinition for static base properties and MagicGameCard for tracking the life-cycle of the in-game card.

@ShawnieBoy
Copy link
Member

I have never understood why there are all these different "card" classes - in my mind you should only need MagicCardDefinition for static base properties and MagicGameCard for tracking the life-cycle of the in-game card.

Does mess with my head on occasion as well - Seeing as the Stack is a zone, treating it like an ordered Graveyard, with abilities being represented as ability-cards kinda makes more sense to me.

I'm sure there's plenty of reasons why though 😀

@lodici lodici mentioned this issue Dec 19, 2015
@lodici
Copy link
Member

lodici commented Dec 19, 2015

@melvinzhang please can you explain how the following methods in MagicItemOnStack return a different MagicCardDefinition given that their implementations appear identical. Thanks.

    @Override
    public MagicCardDefinition getCardDefinition() {
        return getSource().getCardDefinition();
    }

    // only for rendering the card image popup
    final public MagicCardDefinition getRealCardDefinition() {
        return getSource().getCardDefinition();
    }

in StackViewerInfo -

        cardDefinition=itemOnStack.getController().isHuman() ?
            itemOnStack.getRealCardDefinition() :
            itemOnStack.getCardDefinition();

It works, I just can't see how!

@melvinzhang
Copy link
Contributor Author

@lodici These two implementations return the same thing but note that these are just the default implementations. MagicItemOnStack is an abstract class that generalizes spells, triggers, and activated abilities. The individual subclasses can override these methods if they want to do something different.

In particular, MagicMorphCastActivation returns an instance of MagicCardOnStack that overrides getCardDefinition to return the Morph definition.

lodici added a commit that referenced this issue Dec 21, 2015
…opup with modified PT and ability icons (see #567).
@lodici lodici closed this as completed Dec 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants