[WIP] Split Cards Support

[Agetian]

OK, I'm going to start a new topic here now because I'll slowly be returning back to coding Forge, with my first developer target being trying to bring support for split cards (e.g. Fire/Ice) to Forge. Some preparatory research work had been done before my projects went on hiatus last year, but I didn't get far back then and, as far as I can tell, much of the relevant code base has changed since then, so I guess I'm going to start more or less from scratch.

So far, the plan is to:

1. Create a (local?) branch for experiments with split card mechanics. I'm not sure how to properly create and maintain an on-server SVN branch, if anyone is willing to give me a hint or two in that are, you're welcome. Otherwise, for now I'll just set up a secondard copy of the SVN where I'm going to play around with things without committing (at least most of) them to trunk before they're stable enough.

2. As far as I understand, implementing split cards can be subdivided into several subtasks:
a) Implement support for cards that are considered both colors everywhere except on stack (when it goes on stack, only the relevant color should be used).
b) Implement support for Comprehensive rules point 708.5 (more than one answer for a characteristic when queried from a zone other than the stack).
c) Implement support for Comprehensive rules point 708.6 (combined mana cost vs. individual side mana cost distinction in zones other than the stack).
d) Implement proper deck editor support for cards with two mana costs (should probably consider their combined CMC as far as sorting and stuff goes).

Probably the trickiest are "b" and "c" because they have to interact differently with different cards depending on their effect... I'm not sure if this means some of the already existing effects have to be recoded or not.

At any rate, if anybody is willing to join in on this project, even if just by providing valuable input on ideas and feedback about implementation progress, your help is highly welcome. My current schedule is kind of crazy so I won't be able to work too fast/too much for a while, for now I'm gathering ideas and thinking everything over before I can get to implementing (also, doing some preparatory stuff such as SVN branching and source code exploration, too).

[WIP] Classes and methods currently identified to require modification for split card support:

Code: Select all

RELATED TO CARD COLOR:
- None yet

RELATED TO MANA COST / CONVERTED MANA COST:
- Card.getCMC() - make it return the correct CMC (combined/non-combined) for split cards, potentially either by way of "taking a parameter meaning the part of card to evaluate, returning plain number... [that is another update callers quest]" or by way of "return an aggregated value, <...> add some special methods to peek individual card parts which should be used when filtering cards".

RELATED TO OTHER ASPECTS OF RULE ENFORCEMENT:
- None yet

RELATED TO DECK EDITOR SUPPORT:
- None yet

OTHER SIGNIFICANT NOTES:
- None yet


- Agetian

[friarsol]

Hey Agetian,

I've created you a branch here:

http://svn.slightlymagic.net/forge/branches/SplitCards

Basically working with the branch is pretty easy. You work here instead of the trunk, while things are a work in progress, so you don't have to worry about things not being fully functional and other people can help out and see your progress.

Regularly, you'll want the latest changes on the main trunk to be merged into your branch.

When you are done with the feature, we can talk about merging the branch back over to the trunk, no need to overwhelm you with details now.

[Max mtg]

I am planning some changes in CardRules and CardRulesCharacteristics to support split cards.
(eg. add enum SplitType with values = { None, Morph, Flip, Transform, Split }, adjust routines returning CMC of CardRules for split cards - I also hope that deckeditor will handle these cards without any extra efforts at all)

They won't break the game, so these will be applied to trunk. Those will be highly recommended to be merged into the branch.

[Agetian]

Awesome, thank you very much, guys! I'll check out the SplitCards branch and play around a bit to see how to merge from trunk to this branch. I'll also be on the lookout for the upcoming changes to the classes which will be of use. I'll keep you posted, guys! Thanks!

- Agetian

[Agetian]

OK, I've checked out the SplitCards branch and figured out how to merge trunk changes into it. Over the next few days I'm going to go over the current Forge code base and try to locate the critical points in code which will have to be modified for the split cards to work. I'll add a box in the first post here for notes related to that. If you have any comments or recommendations, please let me know.

- Agetian

[RumbleBBU]

Not much help here, just a suggestion...

Once you have the split card functionality in place, one good test would be to see how they interact with Isochron Scepter. The correct behavior, I believe, would be that you can imprint one of the split spells but not both.

And oh, by the way, welcome back, Agetian!

[Sloth]

The correct behavior, I believe, would be that you can imprint one of the split spells but not both.

No. You imprint both sides, but you have to choose which side to play when you activate Isochron Scepter (and yes - you can choose the side that costs more than 2 mana).

[RumbleBBU]

I stand corrected, then.
I remember one rules debate over that issue, should have consulted the Gatherer rulings on that one.

[Max mtg]

I am mostly upset with split card rules.

Key Rule #1: In every zone except the stack, split cards have two sets of characteristics (card type, color, mana cost, etc.).

This is pretty simple to understand for most parts of the card. For instance, Assault // Battery is both red and green, and if you were hit with a Persecute for either color, you'd have to discard Assault // Battery.

It gets tricky when talking about mana costs. Assault // Battery has two mana costs, Red Mana and 3 ManaGreen Mana. It also has two converted mana costs, 1 and 4. The converted mana cost of Assault // Battery is not 5. It is 1 and 4. Similarly, the converted mana cost of Supply // Demand is 2 and 3.

Possible ideas (I didn't estimate them, it's a brain storm):

1. card.getCMC() method returns an array ... that would mean altering every line using that method to add a little loop there

2. card.getCMC() takes a parameter meaning the part of card to evaluate, returns plain number... that is another update callers quest

3. all evaluations are made by predicates which hold the condition to meet and return true/false. They will know how to deal with split cards. The callers will have to be changed to use that predicates.

4. getCMC() returns and aggregated value, but there are still some special methods to peek individual card parts which should be used when filtering cards

5. there's no dedicated getCMC() method, but an enum like CaracteristicsType { Power, Toughness, Color, Type, ManaCost ... } that is passed as parameter to a method getValue(CaracteristicsType, MultiSidesMode ) - MultiSidesMode is also an enum with values like { Aggregated, MainSide, OtherSide }

6 ...

And this project is huge.

[friarsol]

I am mostly upset with split card rules.

[...]

And this project is huge.

Yep, it's a huge pain in the ass, which is why it hadn't been tackled yet.

I think our key points are:

- When things care about total casting cost (and if they do grab all "normal" faces of a card, include Normal characteristic and All Split characteristics) and accumulate them. Ex. Pyromancy, Dark Confidant
- When things care about casting cost of one side of a Split Card (Beseech the Queen)

I think I like option 4 the best. Always return aggregate values, and for the particular cases where we need to consider the split halves separately, check each part of a split card at a time (since it's possible both could be returned as options). Since I picture that as affecting normal cards the least.

[Agetian]

Yeah, it ain't going to be easy, but it "has to be done" (C). Will try to do my best to help out with it. And yeah, I like the fourth option thus far, too, seems like a bit less hassle than the other ones with the same outcome.

- Agetian

[Agetian]

OK, the branch system really seems to hate me... I don't know if it's just me approaching it wrong or something, but I couldn't merge the changes from the trunk today, no matter how hard I tried and what I've done, getting a different kind of error every time. First, I tried to just merge the trunk (went into the Subversion menu, hit Merge, opted to merge from trunk), SVN told me that:

Code: Select all

svn: E195020: Cannot merge into mixed-revision working copy [19,723:19,733]; try updating first


Then, I hit the "Update with Dependencies" button first. No files were updated, but SVN reported:

Code: Select all

update C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards -r HEAD --force
At revision 19767.
==[IDE]== Feb 20, 2013 8:37:40 AM Updating... finished.


After that, I tried the Merge routine again, and this time it did not complain about the necessity to update, but instead failed on the following:

Code: Select all

merge --depth=infinity -r1:HEAD http://svn.slightlymagic.net/forge/trunk C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards
svn: E200014: Checksum mismatch for 'CHANGES.txt'
svn: E200014: Checksum mismatch for 'CHANGES.txt'
svn: E200014: Checksum mismatch for 'CHANGES.txt'
Merge completed abnormally.


Just to make sure it wasn't some random change on my end (which was basically impossible anyway,but still), I hit the "Revert all local modifications" button, then used the "Cleanup Working Copy" to launch "svn cleanup" on my copy, and then tried the merge again. This time it started working but eventually failed anyway with the following:

Code: Select all

........ (a lot of files updated successfully here)
U   C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards/src/main/java/forge/game/zone/PlayerZoneBattlefield.java
U   C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards/src/main/java/forge/game/zone/ZoneType.java
U   C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards/src/main/java/forge/game/GameActionPlay.java
svn: E175002: Could not save file
svn: E175002: Could not save file
svn: E175002: Could not save file
Merge completed abnormally.


Ready to bang my head against the wall, I reverted everything one more time and tried merging again. This time it failed on another file with a different error:

Code: Select all

U   C:/Users/Agetian/Documents/NetBeansProjects/Forge-SplitCards-Branch/SplitCards/src/main/java/forge/game/GameActionUtil.java
svn: E175002: Software caused connection abort: recv failed
svn: E175002: REPORT request failed on '/forge/!svn/vcc/default'
svn: E175002: Software caused connection abort: recv failed
svn: E175002: REPORT request failed on '/forge/!svn/vcc/default'
Software caused connection abort: recv failed
Merge completed abnormally.


This is where I gave up for the time being. Amazingly enough, just updating my copy of the trunk succeeded instantly with no errors or strange complaints. What am I doing wrong here? Like I said, I haven't worked with branches before, so it's likely that I'm just doing something wrong... Please help me if possible. Thanks in advance!

- Agetian

[Max mtg]

Have commited the changes to CardRules I promised. (commited to trunk)
Some class names and visiblity are still subject to change.

Please report any NPEs of similiar wreckage possibly caused by my commit

[friarsol]

I'm not sure why you were having troubles merging, but I just did it and it worked just fine for me.

[Sloth]

@Agetian: Did you try it using TortoiseSVN?