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

Allow non-db cards to be moved around #2960

Merged
merged 2 commits into from
Dec 18, 2017
Merged

Allow non-db cards to be moved around #2960

merged 2 commits into from
Dec 18, 2017

Conversation

ZeldaZach
Copy link
Member

Fixes #2954

Before we could not move cards around if they weren't in the DB b/c if the card was not in the db, the program would discard the entry. There is now an overload variable that tells the program to pretend it's in the database with default values and continue as directed.


Load from Clipboard:
screenshot 2017-12-15 23 51 43

Highlight the row and press "S":
screenshot 2017-12-15 23 51 48

@tooomm
Copy link
Member

tooomm commented Dec 17, 2017

I like that.

What do you think about having the card name in red in the decklist for such cards?
Also, instead default values... it could have a hint in the text or name field to explain why it has no values (or why it's displayed in red), like:
This card does not exist in your card database!

empty

@Daenyth
Copy link
Member

Daenyth commented Dec 17, 2017

I like the idea of painting the background with a css style (so it's themable) defaulting to red when we don't have the card.

That can be a separate PR

@@ -264,11 +264,44 @@ QModelIndex DeckListModel::findCard(const QString &cardName, const QString &zone
return nodeToIndex(cardNode);
}

QModelIndex DeckListModel::addCard(const QString &cardName, const QString &zoneName)
QModelIndex DeckListModel::addCard(const QString &cardName, const QString &zoneName, bool anAddAnyway)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the an in anAddAnyway?

Maybe rename to addIfMissingFromDb?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an = argument variable, n = integer... should be a b = boolean. Will fix this to be
abAddAnyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use hungarian notation, please don't do that. We can tell it's an argument because it's in the argument list and we can tell it's a boolean because it's labelled bool.

@tooomm
Copy link
Member

tooomm commented Dec 18, 2017

I guess we are good here then after the var rename @Daenyth ?

@ZeldaZach ZeldaZach merged commit b75882b into Cockatrice:master Dec 18, 2017
@ZeldaZach ZeldaZach deleted the fix_2954 branch December 18, 2017 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants