Skip to content

Build policy assumption in code #2

@turingfan

Description

@turingfan

The following code is in game_state.legal_moves.cpp and checks for legality of parent card suit.

bool game_state::is_next_legal_card(pol p, card a, card b) const {
    // Checks build pol violations
    switch(p) {
        case sol_rules::build_policy::SAME_SUIT:
            if (b.get_suit() != a.get_suit()) return false;
            break;
        case sol_rules::build_policy::RED_BLACK:
            if (b.get_colour() == a.get_colour()) return false;
            break;
        default:;
    }... 

Notice the default fall through is to pass instead of checking anything. This is ok provided (1) that this code is only called when tableau moves are possible and (2) the three build policies are precisely same-suit, red-black and any suit. This is dangerous as it is a hidden place that new code is necessary if new build policies were added (e.g. different-suit or same-colour).

An alternative would be to add something like

     case sol_rules::build_policy::ANY_SUIT:
            break;
        default: 
            assert(false);   // expecting never to run but avoiding silent errors.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions