search

yourealwaysbe / forkyz

Forkyz Crosswords
GNU General Public License v3.0
39 stars 5 forks source link

Add a "scratch" mode to the play board #14

Closed simonpoole1 closed 3 years ago

simonpoole1 commented 3 years ago

This adds a "Scratch mode" option to the menu on the play board. Anything you type in the play board while in scratch mode goes directly into the scratch field on the notes. Delete works as well, and won't delete non-scratch letters.

Movement logic could definitely be improved in the future to skip over non-scratch letters that have already been entered into the board, but that would required changes to the movement strategies and I thought this PR was already complicated enough. Another future improvement would be to add the same option to the Clue List view.

simonpoole1 commented 3 years ago

Yeah, I think TBH a better model of the game state is needed, encompassing clues, boxes, notes etc. Once I started to make this change it was kind of awkward that Notes are held separately from the answer. Currently the model it's split across Puzzle, Playboard, Box and Notes. Playboard is a mix of game state and game logic. But I think this change is reasonable for now, without adding too much messiness.

An additional change I made in the last commit is to force scratchMode off when on the NotesActivity. If you opened NotesActivity with scratch mode enabled, you were unable to edit the main answer.

matthewhague commented 3 years ago

Yeah, I think TBH a better model of the game state is needed, encompassing clues, boxes, notes etc.

Agreed -- i don't think i really understand what's going on down there and don't really have the heart/energy to attempt a refactor. I probably made it worse with my own changes along the way too. I suspect bringing in Android ViewModels should be part of any refactoring.

An additional change I made in the last commit is to force scratchMode off when on the NotesActivity. If you opened NotesActivity with scratch mode enabled, you were unable to edit the main answer.

Would a better approach be to provide scratch versions of the playLetter and deleteLetter methods, then leave it up to the PlayActivity to decide which one to call? The ClueListActivity would have to do the same. Then scratch mode would be a UI preference rather than a Playboard state that NotesActivity has to workaround?

If NotesActivity is working around it, maybe it could disable it in onResume and restore it in onPause?

simonpoole1 commented 3 years ago

Would a better approach be to provide scratch versions of the playLetter and deleteLetter methods, then leave it up to the PlayActivity to decide which one to call? The ClueListActivity would have to do the same. Then scratch mode would be a UI preference rather than a Playboard state that NotesActivity has to workaround?

This is a good shout - I've made this change.

Also fixed a bug I found when opening a previously opened puzzle - Note objects were being populated for every clue with null values.

yourealwaysbe commented 3 years ago

Looks good -- thanks for all the rewrites!

Possibly scratch mode should be available on the clues list page too, but i think we can call that another feature and add it later if desired.