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

Assertion error in fn Roll during Reserve dice selection #16

Closed danlangford closed 3 years ago

danlangford commented 3 years ago
Assertion failed: (i>0 || m_sides[i]>0), function Roll, file src/bmai.cpp, line 804.

INPUT

game 3
reserve
player 0 8 0
qT
qW
qX
qZ
rnR
rzS
rpU
rfV
player 1 5 0
B8
H8
p8
z8
zU?-30
ply 3
max_sims 100
min_sims 5
maxbranch 400
getaction
quit

OUTPUT

% cat ~/Downloads/71297.txt | ./bmai
BMAI: the Button Men AI
Copyright (c) 2001-2020, Denis Papp.
For information, contact Denis Papp, denis@accessdenied.net
target wins set to 3
s 0.0 Dice (4)0:0 (4)0:0 (4)0:0 (4)0:0
s 0.0 Dice (40)8:0 (10000)8:0 (100)8:0 (10)8:0 (410)30:0
Setting max ply to 3
Setting max # simulations to 100
Setting min # simulations to 5
Setting max branch to 400
l2 p0 Valid SetSwing 45441 Max 80
l2 p0 Valid SetSwing 80 Sims 3
l2 p0 - swing/option T 2  W 4  X 4  Z 4
Assertion failed: (i>0 || m_sides[i]>0), function Roll, file src/bmai.cpp, line 804.

Originally posted by @danlangford in https://github.com/hamstercrack/bmai/discussions/11#discussioncomment-406161

For the record, there are Swing Reserve dice which some have said have been a problem in the past with BMAI

edit: with some recent code changes its now on line 799

l2 p0 Valid SetSwing 45441 Max 80
l2 p0 Valid SetSwing 80 Sims 3
l2 p0 - swing/option T 2  W 4  X 4  Z 4  
Assertion failed: (i>0 || m_sides[i]>0), function Roll, file /Users/danlangford/code/danlangford/bmai/src/bmai.cpp, line 799.

which is here…

void BMC_Die::Roll()
{
    if (!IsUsed())
        return;

    BM_ASSERT(m_state=BME_STATE_NOTSET);

    INT i;
    m_value_total = 0;
    for (i=0; i<Dice(); i++)
    {
        BM_ASSERT(i>0 || m_sides[i]>0); // swing die hasn't been set
        if (m_sides[i]==0)
            break;
        // WARRIOR: always rolls highest number
        if (HasProperty(BME_PROPERTY_WARRIOR))
            m_value_total += m_sides[i];
        else
            m_value_total += g_rng.GetRand(m_sides[i])+1;
    }

    m_state = BME_STATE_READY;

    RecomputeAttacks();
}
danlangford commented 3 years ago

one thing i find interesting is if we look at the call stack the program was going through PlayPreround() and then FinishPreround() but while it was in PlayPreround() for one reason or another it ended up in FinishPreround(). granted by then it could have had a different context but I saw that as possibly odd. (having very little context of the code)

danlangford commented 3 years ago

i am currently exploring a code change in bmai_player.cpp in BMC_Player::RollDice()

currently:

// POST: recomputes m_score
void BMC_Player::RollDice()
{
    //BM_ASSERT(m_man != NULL);

    m_score = 0;

    INT i;
    for (i=0; i<BMD_MAX_DICE; i++)
    {
        if (!m_die[i].IsUsed())
            continue;
        m_die[i].Roll();
        BM_ASSERT(m_die[i].GetValueTotal()>0);

        m_score += m_die[i].GetScore(true);
    }

    OptimizeDice();

    g_logger.Log(BME_DEBUG_ROUND, "rolling ");
    Debug(BME_DEBUG_ROUND);
}

possible change:

// POST: recomputes m_score
void BMC_Player::RollDice()
{
    //BM_ASSERT(m_man != NULL);

    m_score = 0;

    INT i;
    for (i=0; i<BMD_MAX_DICE; i++)
    {
        if (!m_die[i].IsUsed()||m_die[i].GetSides(0)==0)
            continue;
        m_die[i].Roll();
        BM_ASSERT(m_die[i].GetValueTotal()>0);

        m_score += m_die[i].GetScore(true);
    }

    OptimizeDice();

    g_logger.Log(BME_DEBUG_ROUND, "rolling ");
    Debug(BME_DEBUG_ROUND);
}

it helps to make sure that we don't attempt to roll a die that has 0 sides. i don't know what the implications of this are. i don't know how this interacts with twin dice. but so far I actually got the codebase to complete this preround and suggest a reserve selection. so that's good

pappde commented 3 years ago

Ignoring a die where "IsUsed()" but with "m_sides[x]==0" may be masking an underlying issue.

I see in this state, the dice in question is "rnR" or "Reserve Null"

I see in this state, for the die in question: 1) m_state == BME_STATE_NOTSET. As written, RollDice() will ignore any dice set to "NOTUSED" or "RESERVE". 2) m_properties == VALID|NULL 3) m_swing_type == BME_SWING_R

OnUseReserve() must have been called since the PROPERTY_RESERVE is gone. When OnUseReserve() runs, it also sets "STATE_NOTSET"

pappde commented 3 years ago

Here is the full stack:

bmai.exe!BMC_Die::Roll() Line 799 C++ bmai.exe!BMC_Player::RollDice() Line 129 C++ bmai.exe!BMC_Game::FinishPreround() Line 1361 C++ bmai.exe!BMC_Game::PlayRound_EvaluateMove(int _pov_player) Line 2902 C++ bmai.exe!BMC_BMAI3::GetSetSwingAction(BMC_Game _game, BMC_Move & _move) Line 953 C++ bmai.exe!BMC_Game::PlayPreround() Line 1708 C++ bmai.exe!BMC_Game::PlayRound(BMC_Move _start_action) Line 1873 C++ bmai.exe!BMC_BMAI::GetReserveAction(BMC_Game * _game, BMC_Move & _move) Line 1406 C++ bmai.exe!BMC_Parser::GetAction() Line 3646 C++ bmai.exe!BMC_Parser::Parse() Line 3962 C++

In sequence 1) GetReserveAction() runs, 2) starts first simulation 3) ApplyUseReserve() -> which runs OnUseReserve() -> which sets state = NOTSET 4) sim #0: PlayRound() 5) PlayPreround() 6) GetSetSwingAction() 7) ApplySetSwing() 8) PlayRound_EvaluateMove()

I notice at step 8: phasePlayer->GetSwingDiceSet()==SWING_SET_LOCKED, but phasePlayer->NeedsSetSwing()==true.

It looks like the AI has used a reserve die, which happens to be swing, but is (incorrectly) not setting the sides of the swing die.

pappde commented 3 years ago

In GetSetSwingAction(), it will

The assert trips on the very first Move, suggesting it may be invalid. Looking at it:

So it's only setting swing die for index 3 and not setting index 4.

Looking at BMC_Game::GenerateValidSetSwing() note this code:

    // walk dice and determine possible things to set
    for (i=0; i<BME_SWING_MAX; i++)
    {
        if (pl->GetTotalSwingDice(i)>0 && g_swing_sides_range[i][0]>0)
        {
            swing_action[actions].swing = true;
            swing_action[actions].index = i;
            swing_action[actions].value = g_swing_sides_range[i][0];    // initialize to min
            actions++;
            //combinations *= (g_swing_sides_range[i][1]-g_swing_sides_range[i][0])+1;
            move.m_swing_value[i] = g_swing_sides_range[i][0];
        }
    }

That will only handle a swing type if pl->GetTotalSwingDice(i)>0. In this case, for BME_SWING_R, it is returning 0.

INT         GetTotalSwingDice(INT _s) { return m_swing_dice[_s]; }

So m_swing_dice[] is not correct. There are two possible issues here:

A) m_swing_dice[] correctly starts at 0 for reserve dice, and when a swing dice is moved from reserve, it's incorrectly not incrementing m_swing_dice. B) m_swing_dice[] should include counting reserve dice

pappde commented 3 years ago

There are two places in code where m_swing_dice is initialized:

In the first case, m_swing_dice[] will include reserve dice. In the second case, it will exclude them (per IsUsed()), The first code is not actually used, however (it is test code). So only the second case is used. Also, a lot of code checks GetTotalSwingDice(i)>0 and assume that means active swing dice.

SOLUTION OnUseReserve() needs to increment m_swing_dice.

I believe that would fix it, though it's worth noting there's a co-dependency here. A more robust solution would be that whenever a die goes bween"!IsUsed()" and "IsUsed()" (i.e. when m_state changes), it needs to properly update m_swing_dice. E.g. "OnStateChanged()" or "OnIsUsedChanged()"

danlangford commented 3 years ago

Oh man I was going to look at this tonight. Instead I’ll pull your latest and see if it resolves some stuck games of mine

pappde commented 3 years ago

This one was nagging at me since the comment that it's a long standing issue.

danlangford commented 3 years ago

Well I pushed the change to not running against buttonweavers website and it resolved some stale games and is moving forward with wins :-) well done