-
Notifications
You must be signed in to change notification settings - Fork 13
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
undergroundmonorail
wants to merge
3
commits into
mrichards42:master
Choose a base branch
from
undergroundmonorail:space_switch_direction
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
e323a5d
Add "When space is pressed, switch direction" control in preference d…
undergroundmonorail d75bc3d
Add control to OSX preferences panel as well
undergroundmonorail 6c723b1
Factor out function for determining 'opposite' of current focus direc…
undergroundmonorail File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
} | ||
} | ||
|
||
return direction; | ||
} | ||
|
||
puz::Square * | ||
XGridCtrl::SetFocusedSquare(puz::Square * square, | ||
puz::Word * word, | ||
|
@@ -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 | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} | ||
|
||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 theword
argument shouldn't come along for the ride. It looks like in the original context this block of code was concerned with (a) finding aword
if the caller didn't supply one, and (b) figuring out what thedirection
should be.If we want to keep this refactor, I think I'd recommend removing the
word
argument, and then where it's used inSetFocusedSquare
, you'd want to figure out the new word based on the new direction, sodirection = OppositeDirection(square, direction); word = m_puz->FindWord(direction);