4

I have a structural problem for an application I'm working on. In the interest of full disclosure, it is for a university assignment so I'm not looking for you to code me an answer, just help me think through the best approach.

I'm building a game using an OO approach. I have an abstract class called Player from which several types derive. These represent the computer and human players. So I have a derived HumanPlayer class which must provide an implementation for a method called ChooseCard. The way the user selects a card is by clicking on a card in a GUI. So the card will be identified by the user through an event handler.

The relevant part of my code is listed below. At the moment the Game class loops through each player and calls the PlayCard method on each Player. I'm just not sure how to program the ChooseCard method in the HumanPlayer class so that it can wait for the user to select a card to play but have the UI remain responsive. Ideally I want to be able to treat each of the derived player types the same (ie as the base class Player).

This question is asking basically the same thing I am but I don't really like the answers.

The answer to this question is interesting, using the TaskCompletionSource class.

I'm not fixed on this structure if there is a superior solution although I need to keep the class structure.

Edits: Added a class diagram for the models of my application as well as a screenshot of what the gameboard looks like (currently).

Here is my current thinking: The user will click on the card they want to play. The click event will propagate to the bound CardViewModel (At this point the action could be handled at the Model or ViewModel layer). Now the CardViewModel (or Card) doesn't know who is interested in knowing that the user selected it. The interested party will be the PlayerViewModel/HumanPlayer to whom the Card belongs, but they will only be interested when they are expecting a response from the user. I.e. when HumanPlayer.PlayCard() is called.

So I'm thinking that the CardViewModel/Card should raise an event to which the HumanPlayer object can subscribe to when it is interested in the user making a card selection.

I can make HumanPlayer.ChooseCard() subscribe to the CardSelected events of each of the cards. I'm just not sure how to make the method wait without blocking until one of those events fires.

abstract public class Player
{
    public IReadOnlyList<Card> Cards { get { return cards.AsReadOnly(); } }

    public Card PlayCard()
    {
        var cardToPlay = ChooseCard();
        // ...
    }

    protected abstract Card ChooseCard();
}

public class HumanPlayer : Player
{
    protected override Card ChooseCard()
    {
        // TODO: request a card from the user.
        throw new NotImplementedException();
    }
}

public class Game
{
    private List<Player> livingPlayers = new List<Player>();

    public void PlayRound()
    {
        var cardsPlayedByPlayers = GetCardsPlayed();
        // ...
    }

    private Dictionary<Player, Card> GetCardsPlayed()
    {
        var cardsPlayedByPlayers = new Dictionary<Player, Card>();

        foreach (var player in livingPlayers.Where(p => !p.IsDead))
        {
            // Player.PlayCard() called here getting chosen card from the player.
            cardsPlayedByPlayers.Add(player, player.PlayCard());
        }

        return cardsPlayedByPlayers;
    }
}

Update:

I've put the final code up on GitHub here since the assignment is over. You can now see the full code clearly and if you would like to critique it, I'd be keen to hear your thoughts.

  • Are you using WinForms? Or possibly Unity? Unity approaches UI handling a bit differently. Also: are you going to want to simulate ChooseCard() by a bot in the future? That may also impact design here a little bit. – J Trana Sep 29 '14 at 1:41
  • @JTrana At the moment I'm using WPF/XAML since I want to learn that but I didn't want to say which UI technology I was using in case I have to change. But feel free to suggest a WPF only option since I've let the cat out of the bag now. Unity is overkill though. Also since I've talked about the UI, I'm trying to implement the MVVM pattern. – Rossco Sep 29 '14 at 2:38
  • @JTrana Also there will be a number of different bot players implementing different AI. So the ChooseCard method for the bot players will return immediately. But the HumanPlayer will take an appreciable amount of time to make a selection. – Rossco Sep 29 '14 at 2:40
  • The reason I also ask about AI is that you want to make a very clear separation between your theoretical game state and your UI. That is to say, make sure your game model can run completely without UI support - this makes bots, simulating moves ahead, etc. all much easier. In this particular case, I think it may be wise to consider a state machine for the game state. The state machine would then be on its own thread (with appropriate locking etc.) and then the UI or the bot would fire an event that move the state forward. Certainly not the only way to do it, but see if it makes sense. – J Trana Sep 30 '14 at 3:02
2

You have the organization of the code "inside-out". What you're trying to do is respond to a player having chosen a card but instead you're forcing it in the construction of the object which is an awkward place to block (most people don't expect constructors to be "heavy").

Here's your options within the game loop :

1) When evaluating the game loop - do so from a separate thread besides the UI thread and block that thread until the UI has finished choosing a card. This allows you to use 'chooseCard' as a method on the player object - you'd remove it from the constructor and make the method public.

2) If you evaluate the game loop on the main thread then you're not really dealing with player objects but objects that represent the input (arguably you're doing that now already). If so then you should have a delegate or callback implemented that allows the Game loop to react to a "card-chosen" event rather than blocking on a list of objects for each one to complete. Hence, the game loop would maintain in the main thread the state of all the cards chosen and only 'process them' once all participants have responded.

Of the two, the latter is probably more what you're looking for if you aren't a fan of multi-threading.

  • FYI - I didn't include any of the constructors in the provided code as I wanted to focus on the relevant parts of the code. However I'll review the rest of your answer to see what I can glean. – Rossco Sep 30 '14 at 1:15
1

The way you would do it is by using a delegate. Rather than wait for the user input inside the class, delegates allow you to trigger a custom event and have the GUI manage things for you instead.

Below is an outline for setting one up:

  • First define a public delegate so that it is accessible by the GUI.
  • Then in ChooseCard(), add a call to the delegate. This will tell the GUI to handle the specifics of choosing a card.
  • In the GUI, add a reference to the delegate. This part is similar to adding a reference to an event handler.
  • Once the reference is made, you can add a code-block to launch a MessageBox or some sort of notification that would ask the user for their input. Again, this part is similar to event handlers.
  • Finally, when the user makes a selection, pass the data to the Player class.
0

thanks for all your input. It seems the consensus is to use threads to manage things. I currently have a solution that doesn't use threads at the moment and I'd like a critique. I realised that if you were playing the game in real life you need to wait until ALL players are ready to play. Hence I need an IsReady property for each player. Since I'm implementing INPC on basically everything the Game object can be aware of when each player's ready status changes. Here is my code as it stands now:

abstract public class Player
{
    public IReadOnlyList<Card> Cards { get { return cards.AsReadOnly(); } }

    public Card PlayCard()
    {
        if (!IsReady)
        {
            throw new InvalidOperationException("Cannot play card when player is not ready.");
        }

        var cardToPlay = ChooseCard();
        // ...
    }

    public abstract bool IsReady { get; }
    protected abstract Card ChooseCard();
}

public class HumanPlayer : Player
{
    protected override Card ChooseCard()
    {
        // return the selected card.
    }

    public override bool IsReady
    {
        get 
        { 
            // return true if a card is selected.
        }
    }
}

public class Game
{
    private List<Player> livingPlayers = new List<Player>();

    public bool CanPlayRound { 
        get
        {
            // check that all Player.IsReady returns true.
        }
    }

    public void PlayRound()
    {
        var cardsPlayedByPlayers = GetCardsPlayed();
        // ...
    }

    private Dictionary<Player, Card> GetCardsPlayed()
    {
        var cardsPlayedByPlayers = new Dictionary<Player, Card>();

        foreach (var player in livingPlayers.Where(p => !p.IsDead))
        {
            // Player.PlayCard() called here getting chosen card from the player.
            cardsPlayedByPlayers.Add(player, player.PlayCard());
        }

        return cardsPlayedByPlayers;
    }
}
  • It's probably fine for your assignment but in a real company you'd get criticized for "bleeding business logic" and having to "force the workflow to move". A normal game loop is reactive not prescriptive and thus, regardless of entry point should work properly. Here your consumer must call 'IsReady' until it succeeds (and know the next step to make it succeed) or they will get an exception. If it needs to know your workflow you've broken the encapsulation boundaries essentially. – dcgregorya Oct 1 '14 at 20:27
  • I understand what you're saying, however there is no 'game loop'. Each round is initiated manually by the user (by pressing a button which calls PlayRound). The button is only active when all players are ready. Its not perfect but since I'm only supposed to demonstrate that I can use class inheritance I think its going to suffice for the moment. I might post the code to GitHub when the assignment is over. – Rossco Oct 1 '14 at 20:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.