Design Feedback

[Hellfish]

So as I mentioned in my introductory post, I've been going for a while on a Magic engine of my own, and I thought I'd ask you more seasoned Magic programmers for some feedback on the design of the program.

The way it is structured is that there is a Game singleton, that holds everything. It has a list of Player objects, a Stack object, a Layers object and a phase counter.Each Player object has a Deck,Hand,Table,Graveyard and Removed object representing the zones. All of those contain card lists, which all inherits at some level from the CardBase class.

Almost all the rule-enforcement is done by the cards themselves, when inheriting from CardBase, they must override the methods FillOutInfo(),SubscribeToEvents() and CanBePlayed()(Not *must* for SubscribeToEvents(),unfortunately, but they really should)

In FillOutInfo(), the card fills out data about itself, such as Name,Color,Cost,Types.
In SubscribeToEvents(), the card attaches itself as listener to whatever game events it wants to know about from the Game object, which currently has feeds for CardPlayed,CardTapped,CardUnTapped,CardSentToGraveyard(Same as discarded,right?),CardDrawn,PhaseChanged,CardActivated(Think "Clicked on" in say Shandalar). The card provides it's own handler methods for the events, in which it itself has to add an Action to the stack or insert itself into the Layers object for effect processing.

In CanBePlayed(), of course, it determines if it can be played and returns that. It itself has to determine if the player has enough mana for it here (which also led to the unsightly "feature" that you have to tap for enough mana before playing any card..)

Other than those methods, cards have access to the full capabilities of C#, of course.

What remains is creature combat,actually responding to some keywords,finishing the stack and layers and fixing the user interface so that I can actually test what I have in practice >_>

As an example here's the code for Mobile Fort.

Code: Select all

class Mobile_Fort : CardBase
    {
        private bool HasUsedAbilityThisTurn;
        protected override void FillOutInfo()
        {
            Name = "Mobile Fort";
            Color = CardColor.Artifact;
            Cost = new ManaPool(0, 0, 0, 0, 0, 4);
            Types = "Artifact Creature";
            CurPower = OrigPower = 0;
            OrigPower = OrigToughness = 6;
            HasUsedAbilityThisTurn = false;
            Instructions = "Defender (This creature can't attack.)\n3: Mobile Fort gets +3/-1 until end of turn and can attack this turn as though it didn't have defender. Play this ability only once each turn.";
            ShouldUntapAutomatically = true;
            Keywords.Add("Defender");
        }

        protected override void SubscribeToEvents()
        {
            base.SubscribeToEvents();
            Game.Instance.CardActivated += new CardActivatedEventHandler(CardActivated);
            Game.Instance.PhaseChanged += new PhaseChangedEventHandler(PhaseChanged);
        }

        void PhaseChanged(object sender, global::MTGClientSDL.EventArgClasses.PhaseChangedEventHandler e)
        {
            //Remove the powerup effect when we reach end of turn
            if (e.NewPhase == "EndTurn")
            {
                if (HasUsedAbilityThisTurn)
                {
                    Keywords.Add("Defender");
                    HasUsedAbilityThisTurn = false;
                    CurPower -= 3;
                    CurToughness += 1;
                }
            }
        }

        void CardActivated(object sender, global::MTGClientSDL.EventArgClasses.CardEventArgs e)
        {
            //We don't care yet.
            if (e.Card != this || Location != CardLocation.Table)
            {
                return;
            }

            //Controller can't afford ability
            if (!Controller.MyMana.CanPay(new ManaPool(0, 0, 0, 0, 0, 3)))
            {
                Interface.Instance.Alert("Not enough mana. Tap 3 first!");
                return;
            }

            //Controller has already done this once this turn
            if (HasUsedAbilityThisTurn)
            {
                Interface.Instance.Alert("You have already activated this ability this turn.");
                return;
            }

            //Do ability
            Controller.MyMana.Pay(Cost);
            //An Action is just a struct bundling some info together; Description, Function, SourceCard, Type.
            Game.Instance.TheStack.Actions.Push(new Action("Activate Mobile Fort.", new VoidNoParameters(MyStackAction), this, ActionType.Activate));
            HasUsedAbilityThisTurn = true;
        }

        public void MyStackAction()
        {
            //This method will be called by the Stack object in order with other effects and reactions
            CurPower += 3;
            CurToughness -= 1;
            Keywords.Remove("Defender");
        }

        public override bool CanBePlayed()
        {
            return (Owner.MyMana.CanPay(Cost) && (Game.Instance.GetPhase() == "Main1" || Game.Instance.GetPhase() == "Main2"));
        }
    }


If anyone with a bit more experience can foresee any problems with this design, I'd appreciate hearing them.

[frwololo]

- I would add library and stack as zones belonging to each player.

- In the long run, you will want your "mobile fort" card to inherit from other abilities that give +X/+Y until end of turn and gain/loose abilities such as defender. Otherwise you will have lots of code duplication when you create new cards. Probably no need to rush into generic code too soon, but as you add cards you will realize you want them to share as much code as possible.

- Artifact is not a color, be sure to make that clear in your code. It is not in mine, and I run into issues because of that

- I have the same kind of design for power and toughness as you do. In the long run it is not enough. You will want to look at the comprehensive rules for the "layer" system of power/toughness. Actually, most objects in a card should have that kind of layer system to work correctly.An example: there are a few cards that are able to switch their power and toughness. So a 1/3 creature can become a 3/1 creature. but if they get +1/+2, since p and t are exchanged, you actually have to give them +2/+1. I didn't follow this pattern in my original design and now there are cards I can't implement because of that.

- Remember that a "cost" is not always mana. Sometimes it involves sacrifice, discard, tapping other cards, loosing life, etc... You might want your "cost" objects to be as flexible as possible from the beginning.

Actually, I think the best piece of advice you can get from now on is to use the comprehensive rules as your "specifications". That's what I didn't do, and I regret it... a lot.

I know we're all lone wolves with our own projects, but since you seem to handle C++, you might be interested in helping with Wagic in the future
At least my code might give you inspiration of what you could/shouldn't do
http://code.google.com/p/wagic/source/b ... ts/mtg/src

Good luck with your project.

[Hellfish]

Thanks for replying!

Some good points too.
1. I'm not entirely sure why each player should have their own action stack. Seeing as both players can (And frequently will) place actions on the stack following each other on the same phase. (i.e. Your Creature summon->Opponents Counterspell or Your Shock->Opponents Circle Of Protection:Red...) For me this makes it make more sense to have a "neutral", so to speak, stack.

2.I was gonna sleep after that post, but couldn't so now there's a CreatureBase that inherits from CardBase and adds some Creaturespecific stuff. Mobile Fort now inherits from that. However, as I understand it, you're suggesting multiple inheritance, and that's not possible with C# unfortunately.

3. Good point, made it colorless and removed the Artifact color.

4. As far as I've thought about it, the layers seem like a sorted action stack,really. For each event, let all cards that want to add their action to their appropriate layer, sort each layer according to the cards "played" timestamp and execute the actions in order. I'm still milling over this stuff in my head.

5. Yes, but thinking further on that I think the current design works for it too. Say I want to make the Airdrop Condor card.
*I make it listen for the CardActivated event.
**If the activated card in question is that Airdrop Condor card and it is in play untapped without summoning sickness, continue.
**If the required mana hasn't been tapped, abort.
**Allow player to select a player or creature in play. If the player doesn't choose, abort.
**Subtract the mana cost from the players mana pool.
**Send targeted creature to graveyard. (Or rather, put this action on the stack)
**Put the damage action on the stack.

6. Check, gonna go download the newest version right now!

I,hehe, never got much into C or C++. Pointers and explicit memory handling melts my brain C# is much nicer (read: lazier)
I'll help wherever I can,though!

[frwololo]

1. I'm not entirely sure why each player should have their own action stack. Seeing as both players can (And frequently will) place actions on the stack following each other on the same phase. (i.e. Your Creature summon->Opponents Counterspell or Your Shock->Opponents Circle Of Protection:Red...) For me this makes it make more sense to have a "neutral", so to speak, stack.

You're right, it should probably be a shared zone, as long as it is a zone (which was my main point). It could be a zone inheriting from the same class as your other zone objects. You will want to have convenient methods allowing you to move cards from one zone to another.

The only difference the stack has is that it can accept other types of objects (not only cards). So inheritance from a default "zone" class makes perfect sense I think.

[MageKing17]

The stack is the only place copies of spells can exist, so yeah, it's not quite the same as every other zone. It is a zone, however, and should be treated as such.

[malenko]

You're right, it should probably be a shared zone, as long as it is a zone (which was my main point). It could be a zone inheriting from the same class as your other zone objects. You will want to have convenient methods allowing you to move cards from one zone to another.

The only difference the stack has is that it can accept other types of objects (not only cards). So inheritance from a default "zone" class makes perfect sense I think.

One important thing about stack is that user can manipulate it with other spells or ordering the triggered habilities.

[Hellfish]

So I think I've implemented the stack, at least enough for the cards I have now, as well as an idea for how to use it for Counterspell and the like. The stack still only contains actions, but now every Action has a reference to a Source CardBase object. Cards that will exist on the stack will simply be CardBase objects that aren't used anywhere but in the Source reference of an Action. (When the stack resolves, they will place themselves appropriately) The stack "Resolves" by popping each action and calling it's "Function" delegate. Counterspell can only be cast when the last Action on the stack is of type "Cast", and when it resolves it will pop an extra action off the stack, thus removing the spell.

Sorry for length, but I write this for myself as much as anything else

[Incantus]

You've got Counterspell a bit wrong. It can actually target any spell on the stack (imagine players casting a series of instants - you should be able to counterspell any of them)

[Hellfish]

Okay, yeah, I see where you're coming from.So now I've got another method to the Stack class that allows cards to add themselves anywhere in the stack. The cards themselves still do all the adding. So now I need to make the UI work right for it. That is;
1.Let active player plays as many spells as he likes and indicate he's done.
2.Let inactive player respond as much as he wants and indicate.
3. Repeat until both players indicate they don't want to or can't play anything.

You can Counterspell Counterspell s,right?

[Snacko]

217.6 - Stack 217.6a - When a spell is played, the physical card is put on the stack. When an ability is played, it goes on top of the stack without any card associated with it (See Rule 409.1a). CompRules 2007/10/01

217.6b - The stack keeps track of the order that spells and/or abilities were added to it. Each time an object is put on the stack, it's put on top of all objects already there. (See Rule 408, "Timing of Spells and Abilities.") CompRules 2005/08/01

217.6c - If an effect puts two or more objects on the stack at the same time, those controlled by the active player are put on lowest, followed by each other player's objects in APNAP order (see Rule 103.4). If a player controls more than one of these objects, that player chooses their relative order on the stack. CompRules 2009/02/01

217.6d - Each spell has all the characteristics of the card associated with it. Each activated or triggered ability that's on the stack has the text of the ability that created it and no other characteristics. The controller of a spell is the person who played the spell. The controller of an activated ability is the player who played the ability. The controller of a triggered ability is the player who controlled the ability's source when it triggered, unless it's a delayed triggered ability. The controller of a delayed triggered ability is the player who controlled the spell or ability that created it. CompRules 2009/02/01

217.6e - When all players pass in succession, the top (last-added) spell or ability on the stack resolves. If the stack is empty when all players pass, the current step or phase ends and the next begins. CompRules 2009/02/01

217.6f - Combat damage also uses the stack, in the same way as other objects that use the stack. CompRules 2009/02/01

You should really read the rules before implementing a magic game as it makes easier to design a good non hackish framework from the base up.

As per rule 217.6b objects can only be added at the top of the stack, however you can choose the order 217.6c if they are put on the stack simultaneously.

You should also be wary that some types of cards can only be played while the stack is empty this includes Artifacts, Creatures, Enchantments, Lands (those don't use stack), Sorceries and Planeswalkers unless another effect enables them to be played (ex. flash).

212.2a - A player who has priority may play an artifact card from his or her hand during a main phase of his or her turn when the stack is empty. Playing an artifact as a spell uses the stack. (See Rule 409, "Playing Spells and Activated Abilities.") CompRules 2007/10/01

and other 212.*

After all being said you have the order of actions right, but I would stil urge you to read the rules before implementing anything to be on the safe side and not to have to rewrite everything.

Yes you can Counterspell Counterspells.

[malenko]

You can Counterspell Counterspell s,right?

Better: you can use misdirection to redirect a counterspell

[silly freak]

i'd rather not bury activated abilities into the card code (as your CardActivated sounds) but use composition to give a card abilities. think of scripting for the future. it's easy to make an ability object from a string, but to make a card from a series of ability-describing strings is hard, because there are many for one card. additionally, it may hurt the UI if there are more than one activated ability.