Cockatrice: Can't transform `Liliana, Heretical Healer`.

[diogovk]

OS: Linux: Arch Linux

Cockatrice version: Version b2f2e9d (2017-03-07)

---

Can't transform Liliana, Heretical Healer. That is, when I right click, and select Token: Liliana, Defiant Necromancer nothing happens.
If I try to transform Jace, Vryn's Prodigy it works just fine.
The card/token database is updated.

I checked the xml for the card database, and the information is there, and seems fine.

[ZeldaZach]

cc @Psithief

[Psithief]

@ZeldaZach This isn't my department. That's a flip card, I don't touch it.

Also, it works fine on my machine. dab7316 (2017-01-19)

[Nightfirecat]

I actually have an even worse repro: When I attempt to create Token: Liliana, Defiant Necromancer, Cockatrice crashes. I can't reproduce this crash with any of the other flip-walkers. (or other flip cards as far as I can tell) My tokens.xml seems fine as well.

Edit: This is on 32d1c38 2017-03-10, from the last-token bug patch.

Edit2: Of note, this also happens in tonight's Thopter Pie Network release as well.

[Daenyth]

Can you upload your cards.xml and tokens.xml to gist.github.com and link from here please? Any crash details would be good too

[ctrlaltca]

Crash confirmed:

* thread #1: tid = 0x5a7dfb, 0x0000000101d4f98d QtCore`QString::operator=(QString const&) + 13, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
  * frame #0: 0x0000000101d4f98d QtCore`QString::operator=(QString const&) + 13
    frame #1: 0x000000010007c3ce cockatrice`Player::setLastToken(this=0x000000011186d5b0, cardInfo=0x0000000000000000) + 62 at player.cpp:2538
    frame #2: 0x000000010007c626 cockatrice`Player::actCreateRelatedCard(this=0x000000011186d5b0) + 246 at player.cpp:1106
    frame #3: 0x00000001001d9089 cockatrice`Player::qt_static_metacall(_o=0x000000011186d5b0, _c=InvokeMetaMethod, _id=54, _a=0x00007fff5fbf9c90) + 3081 at moc_player_QRWI6QXNSOLWLX.cpp:520
    frame #4: 0x0000000101ec42e1 QtCore`QMetaObject::activate(QObject*, int, int, void**) + 913
    frame #5: 0x00000001011fe57f QtWidgets`QAction::activate(QAction::ActionEvent) + 303
    frame #6: 0x0000000101384fc2 QtWidgets`QMenuPrivate::activateCausedStack(QVector<QPointer<QWidget> > const&, QAction*, QAction::ActionEvent, bool) + 82
    frame #7: 0x000000010138294f QtWidgets`QMenuPrivate::activateAction(QAction*, QAction::ActionEvent, bool) + 543
    frame #8: 0x00000001012480b1 QtWidgets`QWidget::event(QEvent*) + 1681
    frame #9: 0x0000000101389654 QtWidgets`QMenu::event(QEvent*) + 1140
    frame #10: 0x000000010120758d QtWidgets`QApplicationPrivate::notify_helper(QObject*, QEvent*) + 269
    frame #11: 0x000000010120aca0 QtWidgets`QApplication::notify(QObject*, QEvent*) + 9440
    frame #12: 0x0000000101e93544 QtCore`QCoreApplication::notifyInternal2(QObject*, QEvent*) + 164
    frame #13: 0x0000000101207ef7 QtWidgets`QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool) + 871
    frame #14: 0x0000000101268ca5 QtWidgets`QWidgetWindow::handleMouseEvent(QMouseEvent*) + 2325
    frame #15: 0x000000010126794f QtWidgets`QWidgetWindow::event(QEvent*) + 175
    frame #16: 0x000000010120758d QtWidgets`QApplicationPrivate::notify_helper(QObject*, QEvent*) + 269
    frame #17: 0x0000000101209ed2 QtWidgets`QApplication::notify(QObject*, QEvent*) + 5906
    frame #18: 0x0000000101e93544 QtCore`QCoreApplication::notifyInternal2(QObject*, QEvent*) + 164
    frame #19: 0x00000001017e221a QtGui`QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) + 2794
    frame #20: 0x00000001017c949b QtGui`QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 155
    frame #21: 0x00000001068df821 libqcocoa.dylib`QCocoaEventDispatcherPrivate::postedEventsSourceCallback(void*) + 33
    frame #22: 0x00007fff883c98d1 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17

Context:

(lldb) up
frame #1: 0x000000010007c3ce cockatrice`Player::setLastToken(this=0x000000011186d5b0, cardInfo=0x0000000000000000) + 62 at player.cpp:2538
   2535 
   2536 void Player::setLastToken(CardInfo *cardInfo)
   2537 {
-> 2538     lastTokenName = cardInfo->getName();
   2539     lastTokenColor = cardInfo->getColors().isEmpty() ? QString() : cardInfo->getColors().first().toLower();
   2540     lastTokenPT = cardInfo->getPowTough();
   2541     lastTokenAnnotation = settingsCache->getAnnotateTokens() ? cardInfo->getText() : "";
(lldb) p cardIndo
error: use of undeclared identifier 'cardIndo'
(lldb) p cardInfo
(CardInfo *) $0 = 0x0000000000000000
(lldb) up
frame #2: 0x000000010007c626 cockatrice`Player::actCreateRelatedCard(this=0x000000011186d5b0) + 246 at player.cpp:1106
   1103     * then lets allow it to be created via create another
   1104     */
   1105     CardInfo *cardInfo = db->getCard(dbNameFromTokenDisplayName(actionDisplayName));
-> 1106     setLastToken(cardInfo);
   1107 }
   1108 
   1109 void Player::actCreateAllRelatedCards()

[Daenyth]

CardInfo *cardInfo = db->getCard(dbNameFromTokenDisplayName(actionDisplayName));

blargh, getCard can return nullptr. We should really change it to return optional<CardInfo>

[ctrlaltca]

So, the problem has been introduced in #2455 and happens when you create a related card that doesn't exists as a token in the card database.
The token is named "Liliana, Defiant Necromancer (emblem)" and it seems that the "emblem" suffix is causing the card not to be found.
EDIT: sorry, i didn't realize that Liliana, Defiant Necromance and Liliana, Defiant Necromancer (emblem) are 2 different cards.

[Daenyth]

So for one case we can if (cardInfo != nullptr) setLastToken(cardInfo) as a quick stop but perhaps we should change the dbNameFrom.. method so that it's using the 'smart matching' like [[card]] does, and make smart matching allow emblem to be missing? So that smartLookup("foo") should find Foo and Foo! and Foo (emblem)

[Daenyth]

And maybe the token file is wrong too?

[Psithief]

Liliana, Defiant Necromancer is a card side. It is not a token.
Liliana, Defiant Necromancer also creates an emblem. That _is a token_.
The emblem is attached to Liliana, Defiant Necromancer, not Heretical Healer.
How can the token file be wrong if it worked previously?

[Daenyth]

We recently changed a little bit of how token creation works. It sounds like the cards are properly linked and this is just a bug then

[ctrlaltca]

So, heres why the token creation works for Jace and not for Liliana:
Since #2426 the card name is extracted using a regexp that doesn't really work: https://github.com/Cockatrice/Cockatrice/commit/b9cd942308fb547111ff159732d1660eadd388a2#diff-703dd4a6053aed459e325a18a5998d77R1198

For cards like Jace that only create one type of token, the token creation doesn't need to extract the card name from the menu entry, since it's the only "related card" assigned to the card being clicked on: https://github.com/Cockatrice/Cockatrice/commit/b9cd942308fb547111ff159732d1660eadd388a2#diff-703dd4a6053aed459e325a18a5998d77R2440

For cards like Liliana that can create more than one token, the token creation tries to extract the card name from the menu entry, but the regexp fails and the full label "Token: " is returned instead, and no card by that name can be found in the card database, so getCard() returns a null pointer and thus the crash.

To sum it sup, this crash affects all the cards that can create more than one type of tokens.

[tooomm]

To sum it sup, this crash affects all the cards that can create more than one type of tokens.

Oh wow, that's bad.... on the day of release... 👏
@Nightfirecat even reported it early enough, argh!

One more time: +1 for beta ("release candidate") channel!

[ZeldaZach]

@ctrlaltca Do you want to work on a fix for this? I don't know a lot about Regular Expressions so I'm not sure how I can improve upon it except for adding a "strip Token:" case

[tooomm]

We should also inform @MarkyMarkMcDonald

[MarkyMarkMcDonald]

:( Sorry I missed this bug. Thanks for fixing it @ZeldaZach! Do we think it's worth adding some regression testing around this?
Something like "for every card, 'create related tokens', then 'create another token'" and make sure the correct server commands are sent?

[Daenyth]

Getting automated tests around game state will be a good goal but hard to
achieve since it's such a big and complex class. Given how important it is
though we really should

On Sat, Mar 25, 2017, 12:42 PM Mark McDonald [e-mail]
wrote:

:( Sorry I missed this bug. Thanks for fixing it @ZeldaZach
https://github.com/ZeldaZach! Do we think it's worth adding some
regression testing around this?
Something like "for every card, 'create related tokens', then 'create
another token'" and make sure the correct server commands are sent?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/Cockatrice/Cockatrice/issues/2466#issuecomment-289223468,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA5NGHdsrVnaB1tsFDn_g6kTkg195Lqks5rpUPbgaJpZM4MbQDL
.