pappde / bmai

AI to play Button Men, originally developed to interface with the (now deprecated) unofficial online Button Men website.
MIT License
2 stars 1 forks source link

BMAI is Passing on their turn when Pass is not allowed #55

Closed danlangford closed 3 weeks ago

danlangford commented 3 weeks ago

the rules of the game state that if you have a valid attack you are not allowed to Pass your turn.

however in https://www.buttonweavers.com/ui/game.html?game=105095 we see this

game 3 fight player 0 3 28 4:1 d6:2 d10:3 player 1 3 6 p(4,4):4 12:10 X-4:2 ply 2 max_sims 100 min_sims 5 maxbranch 400 surrender off getaction quit

result in

Time: 0.000000 s Sim: 1470 Sims/Sec: 0.000000 Mvs/Sms | 1.0/100.0 | 3.0/50.0 = 15000 l1 p0 best move (10.0 points, 99.8% win) pass stats 2/5-100/400/0.50 Time: 0.000000 s Sim: 1470 Sims/Sec: 0.000000 Mvs/Sms | 1.0/100.0 | 3.0/50.0 = 15000 action pass

even though there is a valid attack of SKILL 4:1 & d10:3 takes p(4,4):4

and then in https://www.buttonweavers.com/ui/game.html?game=105123 we see this

game 3 fight player 0 4 27.5 4:3 d6:3 d10:5 R-11:9 player 1 3 34 20:16 20:10 20:15 ply 2 max_sims 100 min_sims 5 maxbranch 400 surrender off getaction quit

result in

Time: 0.000000 s Sim: 1465 Sims/Sec: 0.000000 Mvs/Sms | 1.0/100.0 | 6.0/33.0 = 19800 l1 p0 best move (2.7 points, 27.3% win) pass stats 2/5-100/400/0.50 Time: 0.000000 s Sim: 1465 Sims/Sec: 0.000000 Mvs/Sms | 1.0/100.0 | 6.0/33.0 = 19800 action pass

even though there is a valid attack of SKILL 4:3 & d6:3 & R-11:9 takes 20:15

danlangford commented 3 weeks ago

here is a slightly simplified input that produces the same results

game fight player 0 3 28 4:1 d6:2 d10:3 player 1 2 6 4:4 12:10 getaction quit

it might have something to do with stealth(d) skill which reads:

Stealth: d
These dice cannot perform any type of attack other than Multi-die Skill Attacks, meaning two or more dice participating in a Skill Attack. In addition, Stealth Dice cannot be captured by any attack other than a Multi-die Skill Attack.

the problem seems to be in or near or related to void BMC_Game::GenerateValidAttacks(BMC_MoveList & _movelist) where it will test Skill Attacks but won't ever add the move, with the SKILL ATTACK, into the moveList

pappde commented 3 weeks ago

Glancing at the code, it must be not testing that particular skill combination (3+1 => 4). So most likely: A) something wrong with die_stack.Cycle() B) the "early out" test is incorrect

It would seem strange to have never run into this before, but another possibility may be some assumptions that loop makes that changed. Will look at it.

pappde commented 3 weeks ago

fixed in pr #56

pappde commented 3 weeks ago

To summarize the issue, there was a bad "early out" optimization when trying all the skill combinations. The condition check was wrong. For example, if the total of all the attacker dice (in this case 6) is <= the largest defender die (in this case 10), then it does not check all combinations. It checks (with 3 dice):

The correct combinations it should check are (following the cycling logic):

The number of missed combinations, of course, is worse the more attacker dice there are. E.g. with 4 dice it drops 2/3rds in this scenario.

Unfortunately, this was a big bug and the fix may have a noticeable impact on compute cost (since there are more valid moves to try) and quality from missing out on high value moves.

It might also be a good idea to test that code further, or at least the cycle class, to make sure it does actually run through all combinations correctly.

danlangford commented 3 weeks ago

Thanks. Ya stepping through the debugger I saw it checking some combinations, then it went and from another die started checking combinations. I observed it never hit the correct combo and was planning on jumping in further sometime this weekend. Looks like the resolution ended up being more thorough than I would have likely attempted.

I really appreciate your time. I hope you didn’t feel pressured to do it. Sometimes I’ll put an issue in the bmai repo just to remind myself or to show other players its on my radar.

Are you still using VS2017?

On Wed, Oct 2, 2024 at 10:08 PM Denis Papp @.***> wrote:

To summarize the issue, there was a bad "early out" optimization when trying all the skill combinations. The condition check was wrong. For example, if the total of all the attacker dice (in this case 6) is <= the largest defender die (in this case 10), then it does not check all combinations. It checks (with 3 dice):

  • Dice 0, 1
  • Dice 0, 1, 2
  • Abort

The correct combinations it should check are (following the cycling logic):

  • Dice 0,1
  • Dice 0,1,2
  • Dice 0,2
  • Dice 1,2

The number of missed combinations, of course, is worse the more attacker dice there are. E.g. with 4 dice it drops 2/3rds in this scenario.

Unfortunately, this was a big bug and the fix may have a noticeable impact on compute cost (since there are more valid moves to try) and quality from missing out on high value moves.

It might also be a good idea to test that code further, or at least the cycle class, to make sure it does actually run through all combinations correctly.

— Reply to this email directly, view it on GitHub https://github.com/pappde/bmai/issues/55#issuecomment-2390475199, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFQDVEIOPMGLFEGPTW7KMLZZS7LFAVCNFSM6AAAAABPG4JHOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJQGQ3TKMJZHE . You are receiving this because you authored the thread.Message ID: @.***>

pappde commented 3 weeks ago

It bothered me that there was an actual bug that was missing valid moves. It was also a good excuse to test the "build instructions".

I did actually use VS2017 for this, and it worked fine, but noticed that GIT_DESCRIBE is blank (i.e. "Version: (blank)").

I just tested with VS2022 cmake and it worked - better, in fact, since GIT_DESCRIBE was working. So the VS2017 dependency notes can be removed.

danlangford commented 3 weeks ago

VS2022 supports a newer version of cmake. I might upgrade the process which will clean up the cmake config and remove some errors.

I’ll do some testing. I also plan to try to implement some very simple skills similar to Maximum. Due to the significance of this fix, and some minor things I have added over the years (and hope to add soon), I would like to entertain the idea of “releasing” and “tagging” something soon as v3.1.

You were “lunatic” on the old site, right? I noticed your fanatic doesn’t have flavor text. Do you recall what it was so we can add it back? Or would you like to make a new flavor text ? I could write something that mentions the AI and its (your) contribution to the community

On Thu, Oct 3, 2024 at 9:01 AM Denis Papp @.***> wrote:

It bothered me that there was an actual bug that was missing valid moves. It was also a good excuse to test the "build instructions".

I did actually use VS2017 for this, and it worked fine, but noticed that GIT_DESCRIBE is blank (i.e. "Version: (blank)").

I just tested with VS2022 cmake and it worked - better, in fact, since GIT_DESCRIBE was working. So the VS2017 dependency notes can be removed.

— Reply to this email directly, view it on GitHub https://github.com/pappde/bmai/issues/55#issuecomment-2391651123, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFQDVHXJ7JICY66YAOVZN3ZZVL6DAVCNFSM6AAAAABPG4JHOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJRGY2TCMJSGM . You are receiving this because you authored the thread.Message ID: @.***>

pappde commented 3 weeks ago

Version: I agree, this deserves a minor version bump. Also, it would be nice to see that on the output header, like "Version 3.1" in addition to the commit tag.

Flavor Text: I was "lunatic", but don't recall setting flavor text.

danlangford commented 3 weeks ago

i thought you might like to know that somebody found another game where BMAI played very sub-optimally but on v3.0-41 it played the most optimal attack which was a skill attack. well done

im planning a few enhancements to the codebase then we can stamp it 3.1. like VS2022, CMake 3.19, C++17

On Thu, Oct 3, 2024 at 10:10 AM Denis Papp @.***> wrote:

Version: I agree, this deserves a minor version bump. Also, it would be nice to see that on the output header, like "Version 3.1" in addition to the commit tag.

Flavor Text: I was "lunatic", but don't recall setting flavor text.

— Reply to this email directly, view it on GitHub https://github.com/pappde/bmai/issues/55#issuecomment-2391805748, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFQDVADKVQHBPJRFBGSSQDZZVUAZAVCNFSM6AAAAABPG4JHOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJRHAYDKNZUHA . You are receiving this because you authored the thread.Message ID: @.***>

pappde commented 3 weeks ago

That's good to hear. I'm still amazed the overall performance was good in the past, but perhaps the failure scenario isn't common enough to drag down the overall stats.

I added issue 57 for an easy optimization on skill attack search. I'm not sure if compute time has been an issue, though.