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

Improved token loading, removed card price code #2670

Merged
merged 2 commits into from
May 5, 2017

Conversation

ctrlaltca
Copy link
Contributor

Related Ticket(s)

Short roundup of the initial problem

When loading a "plain text" deck from a file or clipboard, tokens were not recognized and added to the main deck

What will change with this Pull Request?

Tokens will be correctly added to the "tokens" deck zone.
Additionally, there were a lot of zone codes ("main", "side", "tokens") scattered around the code. They have been replaced by three new constants DECK_ZONE_*.

@ZeldaZach
Copy link
Member

I pushed an additional change to this PR. It removes all traces of card pricing as we haven't used this feature for a long time and it's just scrap and this code was currently doing some stuff with it that was unneeded.

Copy link
Member

@ZeldaZach ZeldaZach left a comment

Choose a reason for hiding this comment

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

Changes work as expected, but someone else should confirm 1 more time to make sure the changes I put in work as well.

@tooomm tooomm changed the title Improved token loading Improved token loading, removed card price code Apr 30, 2017
@ZeldaZach ZeldaZach requested a review from Daenyth April 30, 2017 02:10
@ctrlaltca
Copy link
Contributor Author

I don't get why you added unrelated changes to this PR, instead of creating a new one.

@ZeldaZach
Copy link
Member

I saw you edited some parts of the code that dealt with pricing and I figured it was time to remove it, and I didn't want a merge conflict to arise. I can revert if you'd like and push your changes in and make a new PR later

@ctrlaltca
Copy link
Contributor Author

ctrlaltca commented May 1, 2017

Don't worry, that's fine
Let's wait for @Daenyth 's review and then merge

@ZeldaZach
Copy link
Member

@Daenyth Can you get around to this at some point this weekend?

Copy link
Member

@Daenyth Daenyth left a comment

Choose a reason for hiding this comment

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

Squash the gcc commit manually but leave the "remove price" commit separate from "change token loading" commit please

src/priceupdater.cpp
src/qt-json/json.cpp
src/localclient.cpp
src/qt-json/json.cpp
Copy link
Member

Choose a reason for hiding this comment

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

indent

@ZeldaZach
Copy link
Member

@Daenyth Done, good to go. I'll merge and not squash when it's ready or you can do it now.

@ZeldaZach ZeldaZach merged commit fd3d622 into Cockatrice:master May 5, 2017
@ctrlaltca ctrlaltca deleted the load_tokens branch May 5, 2017 06:36
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