Rule based AI - fehlRule #42
Conversation
…Right now tests are written false, have to be rewritten
…ricks. created many new tests for this 'AI'
…set within trick.play(card, player), this way it's much nicer to handle (especially tests)
There was a problem hiding this comment.
Looking good! The rules implemented seem reasonable and I'm happy we can move on to something that's smarter than playing a random card.
I love the memory idea - that's pretty close to what I had in mind a good while back and I'm happy to see it come to life.
I've left a ton of comments this time - please don't be intimidated. Some things were a little unclear, some introduced inconsistencies to the rest of the codebase, and some had opportunity to make things a little smoother.
I don't want to leave you waiting for this to get merged for longer, so I took the liberty to work on most of the comments I added already. Mostly small refactorings and renaming. The biggest change is probably in what you introduced as the HandUtil. I had a hard time understanding the intent of the code so I shaped it a little differently so that it reveals intent a little more explicitly (at least that's what I think). Tests remained untouched and still pass, so the behavior should be exactly the same.
frontend/src/models/behaviors.js
Outdated
| trick.highestCard().card.value < highest.value && | ||
| highest.value === 11 && | ||
| fehlCards.length < 4 |
There was a problem hiding this comment.
This is quite a statement. Can we capture it in something with a readable name to reveal intent (a variable, a function or a method even)?
There was a problem hiding this comment.
After all changes, this remains the only thing I don't fully understand. Can you explain the logic in full terms to me just so I understand?
To me this reads like this:
- another player opened a trick with a non-trump
- we have to serve non-trump
- we've got a non-trump card that's got a higher value than the highest card in the current trick
- that specific card we have is an ace, by the way
- and we've got less than 4 trumps on our hands
- as a result, we play the highest trump card we can serve - which is always gonna be an ace
This sounds like a lot of very specific conditions and I'm not sure where we're heading with that.
There was a problem hiding this comment.
Okay, so this seems big but is quite easy to explain.
You can only win the trick if you're having the highest card.
As soon as an Ace is played, there no way to win the trick -> play lowest card.
If you don't have an ace to win the trick -> lowest card.
Since we're playing a sharp doko here, we have at most 6 cards of a suit (obviously it's different for hearts, but I don't want to cover everthing in rules).
If you have 4 cards of a suit, there is definately a player that doesn't have the suit (5 cards you know, two players left), so it's not possible to win the trick -> lowest card.
If it's three, there is a chance that the remaining two cards are split into the remaining players hands and you have a chance of winning the trick.
If it's two or even one there is a very high chance to see you winning the trick. This is actually quite common in games I play and therefore this is the move that actually adds the most value to the AI right now.
Obviously we don't count teams into this right now, which is something I want to add later, when affinities are implemented 😁
There was a problem hiding this comment.
I added new tests to explain this better: 8efde77
frontend/src/models/behaviors.js
Outdated
| this.hand = hand; | ||
| } | ||
|
|
||
| getUsefulTrumpForFehlTrick(trick) { |
There was a problem hiding this comment.
"useful" is pretty vague and it took me a while to understand what you're going for. Finding a single, fitting word for the strategy applied here is hard, so maybe we can add some documentation to describe the strategy.
There was a problem hiding this comment.
Oh, and I'm super curious: Why doesn't this return the king of diamonds, ever? Is this considered a bad move or something? If I can use the king of diamonds to win the current trick, why wouldn't I do so?
There was a problem hiding this comment.
We have this "rule": "Niemals unterm Fuchs stechen / Don't trump lower than a fox". I only trump with king of diamonds if chances are very unlikely that somebody might get higher (for example if I'm the last player in the trick). This is a very opinionated move to be honest, but it's quite convincing when you loose youre king or ten of diamonds to a fox, because you wanted to play cheap.
There was a problem hiding this comment.
Ha, I suspected this would be some kind of "nie unterm Fuchs stechen" but then was confused why playing a ten of diamonds (which is of course lower than a Fuchs) would be returned :)
|
|
||
| getBlankAces() { | ||
| let aces = []; | ||
| [suits.clubs, suits.spades, suits.hearts].forEach(suit => { |
There was a problem hiding this comment.
Not a todo, more food for thought: This will bite us once we build playing solo. Mid-term, we should move [suits.clubs, suits.spades, suits.hearts] to a central place (cards.nonTrumpSuits?) so we can set it in one central place if someone plays a solo.
There was a problem hiding this comment.
Very true, but right now solos aren't implemented, therefore I didn't implement something similar. I had this thought too, though 😀 If you implemented it on the go 👍🏻 Otherwise just add a todo an we'll have something to remember where we need to do some little refactoring.
|
Merging this into master since most concerns have been resolved. For the next bit of features let's do new pull requests - if we keep them small, we can get them in fast. We can keep the discussion from the review alive in here, I'd be curious to get your opinion and answers on some of the things I commented on - but there's no reason to keep this out of master any longer. Specifically, if we could discuss these two open issues, that'd be awesome:
|
|
Very happy to see this implemented 😁 I commented all your reviews and hope I explained the reasoning for the moves the AI takes to an understandable degree. |
I implemented different kinds of memory objects for players, which will be used for a Rule based AI.
Based on them, it is possible to create an AI that knows about cards already played. Additionally I changed the cardToPlay() function to instead use the trick, since the trick other necessary information when deciding what to play.
Right now only a fehlRule is implemented, but this covers roughly 30-40% of the game, considering players will try to win fehl Tricks early in the game.
I suggest merging this asap, since it already improves the user experience enormously.
Obviously I will expand this branch and add more functionality in the future.
This is a list of things that come to my mind and I would like to add in the future:
There a few other fixes or improvements (already included).