Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "When space is pressed, switch direction" control #167

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 49 additions & 20 deletions src/XGridCtrl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,32 @@ XGridCtrl::DrawSquare(wxDC & dc, const puz::Square & square, const wxColour & co
//-------------------------------------------------------
// Focus functions
//-------------------------------------------------------
short
XGridCtrl::OppositeFocusDirection(puz::Square * square,
puz::Word * word,
short direction)
{
if (GetGrid()->IsAcrostic())
{
direction = puz::ACROSS;
}
else
{
puz::GridDirection newdir
= puz::IsVertical(direction) ? puz::ACROSS : puz::DOWN;
if (square->HasWord(newdir))
direction = newdir;
else
{
word = m_puz->FindWord(square, newdir);
if (word)
direction = newdir;
Comment on lines +640 to +642
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I like the idea of this refactor, I think this part of the code does too much to really be able to pull it out of its original context.

OppositeDirection(square, direction) makes a lot of sense to me, but the word argument shouldn't come along for the ride. It looks like in the original context this block of code was concerned with (a) finding a word if the caller didn't supply one, and (b) figuring out what the direction should be.

If we want to keep this refactor, I think I'd recommend removing the word argument, and then where it's used in SetFocusedSquare, you'd want to figure out the new word based on the new direction, so

direction = OppositeDirection(square, direction);
word = m_puz->FindWord(direction);

}
}

return direction;
}

puz::Square *
XGridCtrl::SetFocusedSquare(puz::Square * square,
puz::Word * word,
Expand All @@ -644,23 +670,7 @@ XGridCtrl::SetFocusedSquare(puz::Square * square,
else if (! square->HasWord(static_cast<puz::GridDirection>(direction))
&& ! m_puz->FindWord(square, direction))
{
if (GetGrid()->IsAcrostic())
{
direction = puz::ACROSS;
}
else
{
puz::GridDirection newdir
= puz::IsVertical(direction) ? puz::ACROSS : puz::DOWN;
if (square->HasWord(newdir))
direction = newdir;
else
{
word = m_puz->FindWord(square, newdir);
if (word)
direction = newdir;
}
}
direction = OppositeFocusDirection(square, word, direction);
}
}
// Save old state
Expand Down Expand Up @@ -1545,15 +1555,34 @@ XGridCtrl::OnLetter(wxChar key, int mod)
wxASSERT(! IsRebusEntry());

if (static_cast<int>(key) == WXK_SPACE)
SetSquareText(*m_focusedSquare, _T(""));
else
{
if (HasStyle(SWAP_ON_SPACE))
{
const short direction = OppositeFocusDirection(m_focusedSquare, NULL, m_focusedDirection);
SetFocusedSquare(m_focusedSquare, NULL, direction);
}
else
{
SetSquareText(*m_focusedSquare, _T(""));
}
}
else
{
SetSquareText(*m_focusedSquare, key);
}

// Space bar always moves forward one square
if (static_cast<int>(key) == WXK_SPACE)
SetFocusedSquare(m_focusedWord->FindNextSquare(m_focusedSquare, FIND_WHITE_SQUARE), m_focusedWord);
{
if (!HasStyle(SWAP_ON_SPACE))
{
SetFocusedSquare(m_focusedWord->FindNextSquare(m_focusedSquare, FIND_WHITE_SQUARE), m_focusedWord);
}
}
else
{
MoveAfterLetter();
}
Comment on lines -1548 to +1585
Copy link
Owner

@mrichards42 mrichards42 Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably use a refactor now that we're treating SPACE differently in both contexts (which letter to set, and where to move).

Perhaps something like this (pseudocode):

if letter == SPACE:
    if SWAP_ON_SPACE:
        # just swap, no letter
    else:
        # set blank and move to next square, if any
else:
    # enter letter and do the normal move-to-next behavior

}


Expand Down
4 changes: 4 additions & 0 deletions src/XGridCtrl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ enum GridStyle
STRICT_REBUS = 0x0080,
CONTEXT_MENU = 0x0100, // Not yet(?) implemented
SWAP_ON_DCLICK = 0x0200,
SWAP_ON_SPACE = 0x0400,

DEFAULT_GRID_STYLE = PAUSE_ON_SWITCH
| MOVE_AFTER_LETTER
Expand All @@ -71,6 +72,7 @@ enum GridStyle
| CHECK_WHILE_TYPING
| STRICT_REBUS
| SWAP_ON_DCLICK
| SWAP_ON_SPACE
};

enum CorrectStatus
Expand Down Expand Up @@ -167,6 +169,8 @@ class XGridCtrl

bool UnscrambleSolution(unsigned short key);

short OppositeFocusDirection(puz::Square* square, puz::Word * word, short direction);

// Set* functions trigger an event.
// Move* functions check BLANK_ON_NEW_WORD
// Essentially this is SetFocusedSquare([square, ][word, ][direction])
Expand Down
8 changes: 8 additions & 0 deletions src/dialogs/PreferencesPanel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ void SolvePanel::DoLoadConfig()
m_nextSquare->SetValue(1);
m_blankOnDirection->SetValue((gridStyle & BLANK_ON_DIRECTION) != 0);
m_blankOnNewWord ->SetValue((gridStyle & BLANK_ON_NEW_WORD) != 0);
if (gridStyle & SWAP_ON_SPACE)
m_switchDirectionsOnSpace->SetValue(1);
else
m_insertBlankOnSpace->SetValue(1);
m_pauseOnSwitch ->SetValue((gridStyle & PAUSE_ON_SWITCH) != 0);
m_moveOnRightClick->SetValue((gridStyle & MOVE_ON_RIGHT_CLICK) != 0);
m_swapOnDClick ->SetValue((gridStyle & SWAP_ON_DCLICK) != 0);
Expand Down Expand Up @@ -253,6 +257,8 @@ void SolvePanel::DoSaveConfig()
gridStyle |= BLANK_ON_DIRECTION;
if (m_blankOnNewWord->GetValue())
gridStyle |= BLANK_ON_NEW_WORD;
if (m_switchDirectionsOnSpace->GetValue())
gridStyle |= SWAP_ON_SPACE;
if(m_pauseOnSwitch->GetValue())
gridStyle |= PAUSE_ON_SWITCH;
if (m_moveOnRightClick->GetValue())
Expand Down Expand Up @@ -284,6 +290,8 @@ void SolvePanel::ConnectChangedEvents()
BindChangedEvent(m_nextBlank);
BindChangedEvent(m_blankOnDirection);
BindChangedEvent(m_blankOnNewWord);
BindChangedEvent(m_insertBlankOnSpace);
BindChangedEvent(m_switchDirectionsOnSpace);
BindChangedEvent(m_pauseOnSwitch);
BindChangedEvent(m_moveOnRightClick);
BindChangedEvent(m_swapOnDClick);
Expand Down
16 changes: 16 additions & 0 deletions src/dialogs/wxFB_PreferencesPanels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ wxFB_SolvePanel::wxFB_SolvePanel( wxWindow* parent, wxWindowID id, const wxPoint

sbSizer3->Add( bSizer13, 0, wxLEFT, 20 );

m_staticText11 = new wxStaticText( sbSizer3->GetStaticBox(), wxID_ANY, wxT("When pressing space"), wxDefaultPosition, wxDefaultSize, 0 );
m_staticText11->Wrap( -1 );
sbSizer3->Add( m_staticText11, 0, wxALL, 5 );

wxBoxSizer* bSizer14;
bSizer14 = new wxBoxSizer( wxVERTICAL );

m_insertBlankOnSpace = new wxRadioButton( sbSizer3->GetStaticBox(), wxID_ANY, wxT("Insert a blank"), wxDefaultPosition, wxDefaultSize, 0 );
bSizer14->Add( m_insertBlankOnSpace, 0, wxALL, 5 );

m_switchDirectionsOnSpace = new wxRadioButton( sbSizer3->GetStaticBox(), wxID_ANY, wxT("Switch directions"), wxDefaultPosition, wxDefaultSize, 0 );
bSizer14->Add( m_switchDirectionsOnSpace, 0, wxALL, 5 );


sbSizer3->Add( bSizer14, 0, wxLEFT, 20 );

m_pauseOnSwitch = new wxCheckBox( sbSizer3->GetStaticBox(), wxID_ANY, wxT("Pause when switching direction"), wxDefaultPosition, wxDefaultSize, 0 );
m_pauseOnSwitch->SetValue(true);
sbSizer3->Add( m_pauseOnSwitch, 0, wxALL, 5 );
Expand Down
Loading