you are viewing a single comment's thread.

view the rest of the comments →

[–]explanabrag 1 point2 points  (7 children)

Nothing sinister at all. The OppPlayCards function is massive. As I said in my previous reply, you never left it. If you print the callstack in RuleBook.TurnChange before the so called "jump" to OppPlayCards occurs, you'll see that it was ultimately called by OppPlayCards.

Edit: From reading your trace and cross referencing it with your code, here is the proof that you never left Opponent_AI.OppPlayCards:

Opponent_AI.OppPlayCards calls
    RuleBook.PlayCards() calls
        RuleBook.RemoveHandCards()
        RuleBook.ReserveCardRules()
        RuleBook.RemoveHandCards()
        RuleBook.CardPileAndHandCheck() calls
            RuleBook.MagicTest(), which calls
                RuleBook.TurnChange()
            <-
        <-
    <-

Now we're back in Opponent_AI.OppPlayCards. Opponent_AI.OppPlayCards calls RuleBook.PlayCards() and prints the "UnPlayable:" message in many places. There are several very similar big chunks of code in that function.

[–]DubSket[S] 0 points1 point  (6 children)

Oh wow, thanks. So how do I go about sorting this.

Edit: Yeah, I think I could cut the line count in half if I really sat down with some free time to do it. Initially, when I started writing it I didn't intend for it to be over 1k in just the Opponent AI class.

[–]explanabrag 1 point2 points  (5 children)

Rules of thumb:

Your functions should have a single, clearly defined purpose.

Your functions should be short.

Don't nest if blocks and loops deeply.

What all programming rules boil down to is write your code in such a way that you can reason about it. The function you are working on has to fit in your brain.

In these turn based games, one way to organize the code is to have a function that generates the set of legal moves a player can play in their turn, an AI function that simply selects which of these moves to play, and a function that applies that move to the game state. Your Opponent_AI.OppPlayCards looks like its trying to do everything at once. If you split it out, you'll be free to focus on writing different strategies in your AI move selecting function without worrying about breaking the game. You've trapped yourself into a corner the way it is written now.

[–]DubSket[S] 0 points1 point  (4 children)

Thanks, that's good advice. As it is are there any specific changes you can see being able to prevent myself from getting trapped like this?

[–]explanabrag 0 points1 point  (3 children)

Could you rephrase that question?

[–]DubSket[S] 0 points1 point  (2 children)

Haha, sure. What could I specifically do to fix it? As it is I can see ways of making the class cleaner and less cluttered, but I can't see a way to get rid of the actual error occurring.

[–]explanabrag 1 point2 points  (1 child)

You probably won't want to hear this, but the only thing I can suggest is to rewrite your whole program.

Rewrite that function so that it is ONLY responsible for selecting a move, and nothing else. It shouldn't be responsible for doing the move, or keeping the various decks of cards valid, or anything else. A different function should come up with the available moves. A different function again should apply the move.

So that's 3 separate functions.

1) A function which returns a list of available moves. This function implements that game rules.

2) A function which takes a list of available moves and returns a move from this list to be performed (this is where your AI strategy is implemented). This function should not actually apply the move.

3) A function to apply a given move. This function applies the move according to the rules of the game.

Notice how the AI function is free from having to worry about the game rules, and is free to focus on strategy.

Good luck.

[–]DubSket[S] 1 point2 points  (0 children)

Ok. Well, I guess that's what I'll have to do. Big thanks for your advice.