Player as its own Class

[slapshot5]

Something that's been in the back of my head for awhile:

Should Player be its own class now, instead of a String?

Just from past experience, I feel like there are cards I haven't implemented because you need info about the Player that is not easily accessible in the current structure.

So, Owners/Controllers would be checked for Player instead of String (i.e.):

Code: Select all

if(c.getController().isHuman()

Also, things like Words of Worship should be easier. Preventing damage to a player should be easier with things like Alabaster Potion, Amulet of Kroog.

Would this be beneficial at this point?

Please post your thoughts/implementation ideas.

-slapshot5

[Rob Cashwalker]

Abso-freaking-lutely!

But it's going to be a helluva lot of work to make that happen.... Though I suppose if you simply remove the controller/owner strings from the card object, it should cause enough error highlights to find everywhere in the entire architecture the player class would need to be applied.
For example, it's used even in the deck builders.. this is on the scale of what dennis had to do when protection was first added, but back then there were only like 600 CODED cards. There were a total of about 1400, I think, which was primarily made up of my 3 or 4 hundred card submission of all vanilla and french vanilla creatures that we could support at the time.

[Sloth]

I think it would be a good idea.

I'm thinking about temporary abilities a player can gain like with Gilded Light, Seht's Tiger and Angel's Grace.

A player class would be the natural place for them to be stored.

[slapshot5]

Ok. I'll see if I can't whip something up this week. Better to do it when we have 5000 cards than when we have 6000, right? I probably won't commit anything that changes default behavior until after the next beta. I'll probably just make a new local repo for now. That should give us the most testing time since we're probably halfway into the next beta by now (assuming early November?).

If you have any implementation details, or methods you would like implemented, please add to this thread.

-slapshot5

[Rob Cashwalker]

Even just checking and manipulating a life total will be simpler.

dumb-looking lines of code that what we have now:
setTargetPlayer(AllZone.GameAction.getOpponent(card.getController()));
new CardList(AllZone.getZone(Constant.Zone.Play, Constant.Player.Human).getCards());

stream-lined lines of code we could have:
setTargetPlayer(card.getController().getOpponent());
new CardList(AllZone.HumanPlayer.getCards(Constant.Zone.Play))

In theory, if this were implemented to the full extent, then the concept of AI vs AI should become a very simple extension.

[friarsol]

Things a player has/can gain:

Life, Poison Counters, Protection, Shroud, Can't play Spells/Abilities, Prevent Damage.

It would be good if all these things were tied into the same object. I know there exists a NullPlayerCard for each player, this should probably be replaced as the entry point for a new class.

[DennisBergkamp]

Yes, this is good stuff... the coolest thing this would allow is to implement cards that give the player shroud (which would require probably require some work, since we currently don't have any target checking for players coded).

[Rob Cashwalker]

Damage and damage modification for both cards and players should be moved to the card and player objects, instead of the GameAction.addDamage method.

[slapshot5]

Here's a quick and dirty off the top of my head. I'm sure the implementation could be a bit cleaner, but getting it to work is going to be priority No. 1.

Code: Select all


package forge;


public class Player extends MyObservable{
    private String name;
    private int poisonCounters;
    private int life;
   
    public Player(String myName) {
        this(myName, 20, 0);
    }
   
    public Player(String myName, int myLife, int myPoisonCounters) {
        name = myName;
        life = myLife;
        poisonCounters = myPoisonCounters;
    }
   
    public String getName() {
        return name;
    }
   
    public boolean isHuman() {
        return name.equals("Human");
    }
   
    public boolean isComputer() {
        return name.equals("Computer");
    }
   
    public boolean isPlayer(Player p1) {
        return p1.getName().equals(this.name);
    }
   
    public Player getOpponent() {
        if(isPlayer(AllZone.HumanPlayer)) {
            return AllZone.ComputerPlayer;
        }
        else return AllZone.HumanPlayer;
    }
   
    //////////////////////////
    //
    // methods for manipulating life
    //
    //////////////////////////
   
    public boolean setLife(final int newLife) {
        life = newLife;
        return true;
    }
   
    public int getLife() {
        return life;
    }
   
    public boolean gainLife(final int toGain) {
        boolean newLifeSet = false;
        if(toGain > 0) {
            life += toGain;
            newLifeSet = true;
        }
        else System.out.println("Player - trying to gain negative or 0 life");
        return newLifeSet;
    }
   
    public boolean loseLife(final int toLose) {
        boolean newLifeSet = false;
        if(toLose > 0) {
            life -= toLose;
            newLifeSet = true;
        }
        else System.out.println("Player - trying to lose positive or 0 life");
        return newLifeSet;
    }
   
    //////////////////////////
    //
    // methods for handling Poison counters
    //
    //////////////////////////
   
    public void addPoisonCounters(int num) {
        poisonCounters += num;
        this.updateObservers();
    }
   
    public void setPoisonCounters(int num) {
        poisonCounters = num;
        this.updateObservers();
    }
   
    public int getPoisonCounters() {
        return poisonCounters;
    }
   
    public void subtractPoisonCounters(int num) {
        poisonCounters -= num;
        this.updateObservers();
    }
   
   
   
    public boolean canPlaySpells() {
        return true;
    }
   
    public boolean canPlayAbilities() {
        return true;
    }
   
    public CardList getCards(PlayerZone zone) {
        //TODO
        return new CardList();
    }
   
    public String toString() {
        return name;
    }
}


Obviously, it's not complete, and what's there isn't fully implemented. I'll have to pull some of the implementations out of GameAction.java.

-slapshot5

[slapshot5]

I have a first pass done for Player.java and updating all code to use the new class.

Player.java:

Code: Select all


package forge;


public class Player extends MyObservable{
    private String name;
    private int poisonCounters;
    private int life;
    private int assignedDamage;
   
    public Player(String myName) {
        this(myName, 20, 0);
    }
   
    public Player(String myName, int myLife, int myPoisonCounters) {
        name = myName;
        life = myLife;
        poisonCounters = myPoisonCounters;
        assignedDamage = 0;
    }
   
    public String getName() {
        return name;
    }
   
    public boolean isHuman() {
        return name.equals("Human");
    }
   
    public boolean isComputer() {
        return name.equals("Computer");
    }
   
    public boolean isPlayer(Player p1) {
        return p1.getName().equals(this.name);
    }
   
    public Player getOpponent() {
        if(isPlayer(AllZone.HumanPlayer)) {
            return AllZone.ComputerPlayer;
        }
        else return AllZone.HumanPlayer;
    }
   
    //////////////////////////
    //
    // methods for manipulating life
    //
    //////////////////////////
   
    public boolean setLife(final int newLife) {
        life = newLife;
        this.updateObservers();
        return true;
    }
   
    public int getLife() {
        return life;
    }
   
    public void addLife(final int toAdd) {
        life += toAdd;
        this.updateObservers();
    }
   
    public boolean gainLife(final int toGain) {
        boolean newLifeSet = false;
        if(toGain > 0) {
            addLife(toGain);
            newLifeSet = true;
        }
        else System.out.println("Player - trying to gain negative or 0 life");
       
        /*
        Object[] Life_Whenever_Parameters = new Object[1];
        Life_Whenever_Parameters[0] = toGain;
        AllZone.GameAction.CheckWheneverKeyword(p.getPlayerCard(), "GainLife", Life_Whenever_Parameters);
        */
        return newLifeSet;
    }
   
    public boolean loseLife(final int toLose) {
        boolean newLifeSet = false;
        if(toLose > 0) {
            life -= toLose;
            newLifeSet = true;
        }
        else System.out.println("Player - trying to lose positive or 0 life");
        return newLifeSet;
    }
   
    public void subtractLife(final int toSub, final Card c) {
        life -= toSub;
        this.updateObservers();
    }
   
    public void payLife(final int cost) {
        life -= cost;
    }
   
    public boolean payLife(int lifePayment, Card source) {
       
        if (lifePayment <= life){
            subtractLife(lifePayment, source);
            return true;
        }
        return false;
    }
   
    //////////////////////////
    //
    // methods for handling damage
    //
    //////////////////////////
   
    public void addDamage(int damage, Card source) {
        if (source.getKeyword().contains("Infect")) {
            addPoisonCounters(damage);
        }
        else {
            int damageToDo = damage;
            /*if(worshipFlag(player) && life <= damageToDo) {
                damageToDo = Math.min(damageToDo, life - 1);
            } */
            subtractLife(damageToDo,source);
        }
           
        if(source.getKeyword().contains("Lifelink")) GameActionUtil.executeLifeLinkEffects(source, damage);
       
        CardList cl = CardFactoryUtil.getAurasEnchanting(source, "Guilty Conscience");
        for(Card c:cl) {
            GameActionUtil.executeGuiltyConscienceEffects(source, c, damage);
        }
       
        GameActionUtil.executePlayerDamageEffects(this, source, damage, false);
    }
   
    public void setAssignedDamage(int n)           {    assignedDamage = n; }
    public int  getAssignedDamage()                {    return assignedDamage; }
   
   
   
   
   
    public void addCombatDamage(int damage, final Card source) {
        if (source.getKeyword().contains("Prevent all combat damage that would be dealt to and dealt by CARDNAME.")
                || source.getKeyword().contains("Prevent all combat damage that would be dealt by CARDNAME."))
            damage = 0;
        if (source.getKeyword().contains("Infect")) {
            //addPoison(player, damage);
        }
        else {
            addDamage(damage, source);
        }
       
        //GameActionUtil.executePlayerDamageEffects(player, source, damage, true);
        GameActionUtil.executePlayerCombatDamageEffects(source);
        CombatUtil.executeCombatDamageEffects(source);
    }
   
    //////////////////////////
    //
    // methods for handling Poison counters
    //
    //////////////////////////
   
    public void addPoisonCounters(int num) {
        poisonCounters += num;
        this.updateObservers();
    }
   
    public void setPoisonCounters(int num) {
        poisonCounters = num;
        this.updateObservers();
    }
   
    public int getPoisonCounters() {
        return poisonCounters;
    }
   
    public void subtractPoisonCounters(int num) {
        poisonCounters -= num;
        this.updateObservers();
    }
   
    public boolean hasShroud() {
        return false;
    }
   
   
    public boolean canPlaySpells() {
        return true;
    }
   
    public boolean canPlayAbilities() {
        return true;
    }
   
    public CardList getCards(PlayerZone zone) {
        //TODO
        return new CardList();
    }
   
    public String toString() {
        return name;
    }
}


full source will all changes can be found here:
http://www.bunker-ranch.org/~denny/mtg-forge/src.7z

It's basically functional, though I'm sure there are a few bugs. Probably will run into a few NullPointerExceptions.

Here's what I'd like to do:
1) in another thread, I think Chris mentioned possibly posting a beta this weekend. Can we base that off the current SVN rev (even if it doesn't get posted til Saturday)?
2) if so, I'd like to check in what I have to start a fresh beta cycle. That way we have max time to squash bugs before the next beta. This is a pretty invasive change.

Reason being is that the further my code tree diverges for HEAD in SVN, the more impossible it will be to merge with any new changes. There are literally thousands of one-line changes.

Thoughts?

Hopefully this will open Forge up even more to cards previously thought impossible.

-slapshot5

[Sloth]

Thoughts?

I have some time to test today. So I wouldn't mind you commiting it to the SVN.

[friarsol]

1) in another thread, I think Chris mentioned possibly posting a beta this weekend. Can we base that off the current SVN rev (even if it doesn't get posted til Saturday)?

Actually, I think this is a good idea. Last release we had a ton of submissions up to the last second of the release being created, and those cards didn't really get to be tested by everyone else, leading to a possibility of some bugs that just sit in that release, or if we create a patch might be part of the patch. If we make a cut-off for what revision will be the release revision, there won't be this seem issue of a last minute push. Those cards will still go in the next release but we'll have plenty of time to notice any issues.

[slapshot5]

This has now been committed. Please test and note any bugs in this thread.

Feel free to flesh stuff out more as needed or as you see fit.

r2833.

-slapshot5

[Sloth]

This has now been committed. Please test and note any bugs in this thread.

Feel free to flesh stuff out more as needed or as you see fit.

r2833.

-slapshot5

There are still nullpointer exceptions with the whenever keyword. I will take a look at this monster.

Edit: I tested Snake Umbra.

[Rob Cashwalker]

At some point it might be good to make Player abstract, and then have two sub-classes - HumanPlayer and AIPlayer.

For example - a Player may be given a game-command of discarding or sacrificing a card. The code that gave that command doesn't have to do anything special if that Player.isHuman or that Player.isComputer. Consider the GameAction.discard code... it deals with human and computer by passing the work off to different methods contained in GameAction, which is getting quite messy. The originating code only needs to call something like tgtPlayer.Discard(num, Options).

Why should this matter? Because it would open the door for the possibility that Player1 may be EITHER human or computer. Initially, this just cleans up the code a bit. But then consider how it would enhance the turn structure - no more jumping into the middle so that the computer goes first. Also, how about the player has methods like doUpkeep, doMain1, doAttack, etc. The human class would prompt the user for input, while the computer class calls upon the AI classes we already use.