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

braces are only left for split cards #3217

Merged

Conversation

Vafthrudnir
Copy link
Contributor

Short roundup of the initial problem

Due to a mistake in #3108 all mana costs are shown in braces instead of only split costs. This PR fixes that.

Screenshots

image

@ZeldaZach
Copy link
Member

Thought this was a feature. Whoops haha

@Vafthrudnir
Copy link
Contributor Author

Well, the previous modification was only meant to put braces around split mana costs, but if you think it's better I can change it back to that version.

The code must be modified either way, because in that case it's quite useless to look for split costs and then do nothing special with them.

ctrlaltca
ctrlaltca previously approved these changes May 2, 2018
Copy link
Contributor

@ctrlaltca ctrlaltca left a comment

Choose a reason for hiding this comment

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

EDIT: see @tooomm 's comment

Copy link
Member

@tooomm tooomm left a comment

Choose a reason for hiding this comment

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

For split cards it's not working as expected:
split

@ctrlaltca ctrlaltca dismissed their stale review May 2, 2018 16:05

agreed with @tooomm's

@Vafthrudnir
Copy link
Contributor Author

Thanks for noticing that @tooomm! Fixed it in the previous commit.

ZeldaZach
ZeldaZach approved these changes May 2, 2018
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.

Still an issue

@ZeldaZach
Copy link
Member

screenshot 2018-05-02 18 08 23

screenshot 2018-05-02 18 08 28

screenshot 2018-05-02 18 08 49

@Vafthrudnir
Copy link
Contributor Author

Have you run oracle again? This same version show up as this at me:
image
image

@ZeldaZach ZeldaZach merged commit 9727699 into Cockatrice:master May 3, 2018
@Vafthrudnir Vafthrudnir deleted the hotfix/3108_braces_for_split_costs branch May 9, 2018 17:24
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

4 participants