In grid-snapping mode, Untangle was still recording vertex positions
in increments of one pixel. This meant that if you positioned
vertices on a small window, then enlarged the window and positioned
more vertices, the two sets of vertices generally wouldn't line up
with one another. This was annoying, and obviously silly when
Untangle has a resolution-independent co-ordinate system. So now the
snapped positions are recorded in a form that doesn't depend on the
tilesize.
If you clicked somewhere that had no effect (outside the grid or on a
square matching the colour of the top-left corner), interpret_move()
would return MOVE_NO_EFFECT (previously NULL) even though it had unset
ui->cursor. So the keyboard cursor would remain visible until the
next window resize (or similar) when it would vanish. Now
interpret_move() correctly returns MOVE_UI_UPDATE in these cases, so
the cursor vanishes immediately.
If you clicked somewhere that had no effect (on an immutable square or
a middle click on an empty square), interpret_move() would return NULL
even though it had unset ui->cursor. So the keyboard cursor would
remain visible until the next window resize (or similar) when it would
vanish. Now interpret_move() correctly returns MOVE_UI_UPDATE in
these cases, so the cursor vanishes immediately.
I missed this in my main commit for UI preferences
(4227ac1fd5dc25c247e7526526079b85e1890766) because it wasn't documented.
Now it is documented and it has a preference.
As far as I could tell, cur_x, cur_y, prev_cur_x, and prev_cur_y in
game_drawstate were never used. prev_cur_x and prev_cur_y in game_ui
were assigned but never referenced. So they may as well all go.
It looks as if it's been broken for about nine years, ever since
commit 822243de1bc1fc6d introduced the system for drag-selecting a
diagonal of squares. The effect of moving the keyboard cursor and then
pressing a button was to cause crashes because the ui fields
introduced for that system to use (ohx, ohy, odx, ody, odn) were all
completely uninitialised.
If you're using the mouse to change pencil marks, you have to
right-click to pencil-highlight a square, then press a number or
letter key to add or remove a highlight. That causes the highlight to
vanish again. So adding or removing multiple pencil marks requires a
right-click + keypress per mark.
Chris's Android port reversed that decision, making the pencil
highlight persist so that you could 'click' just once and then press
multiple pencil keys. That makes it easier to add lots of highlights,
but harder to just remove a single one (click + press + click to
remove the highlight), unless you don't mind keeping the highlight
around afterwards cluttering up your view.
In other words, this is just the sort of thing users might reasonably
disagree on. So now we have an organised preferences system, we can
let them disagree, and each configure it whichever way they like!
This only affects mouse-based play. The keyboard cursor has _always_
worked this way, because it doesn't disappear at all; its behaviour is
unchanged, and independent of the new preference.
Light Up is unusual in that clicking outside the grid hides the
cursor, so the return value from clicks outside the grid is
MOVE_NO_EFFECT or MOVE_UI_UPDATE rather than the more usual
MOVE_UNUSED.
This adds an extra parameter to move_cursor() that's an optional pointer
to a bool indicating whether the cursor is visible. This allows for
centralising the common idiom of having the keyboard cursor become
visible when a cursor key is pressed. Consistently with the vast
majority of existing puzzles, the cursor moves even if it was invisible
before, and becomes visible even if it can't move.
The function now also returns one of the special constants that can be
returned by interpret_move(), so that the caller can correctly return
MOVE_UI_UPDATE or MOVE_NO_EFFECT without needing to carefully check for
changes itself.
Callers are updated only to the extent that they all pass NULL as the
new argument. Most of them could now be substantially simplified.
This means that it now potentially overlaps the peg above it (part of
the current guess), rather than potentially overlapping the empty hole
below. More importantly, it means that the hold marker is erased by
the erasure of the rest of the peg area, so there's no need to
explicitly draw absent hold markers in the background colour. That in
turn means that absent hold markers don't nibble the tops off all the
pegs at some tile sizes.
Instead of this fix, I could have properly made the hold markers part
of the first row of empty holes, but that would have been rather
fiddly and I've long thought that the hold markers were too far from
the peg that they're holding.
I've also removed part of a comment about the drawing order of hold
markers that seems to have been obsolete even before this commit.
Pearl generally has to generate quite a lot of candidate loops before
it can find one that makes a viable puzzle. Before this change it
generated a new grid structure for each of those candidate loops. The
result was that grid_new() accounted for over 5% of the
puzzle-generation time.
Pulling grid_new() out of the loop-generation loop makes "pearl
--generate 100 8x8dt#0" about 6% faster on my laptop, while producing
precisely the same output. Most of this change is just renaming the
"grid" variable in new_clues() so it doesn't collide with the typedef
of the same name.
Same Game doesn't want to show the keyboard cursor when the game is in a
state where no move is possible. Previously, it did this by having
game_changed_state() hide the cursor on entry to such a state. That
meant that reaching a dead end and undoing out of it hid the cursor,
which was confusing.
Now the cursor is hidden in game_redraw() if the game is in a dead-end
state without changing the displaysel flag in the game_ui. That way, if
you undo out of a dead end, the cursor becomes visible again if it was
visible before.
This does mean that you can move the cursor in a dead-end state without
being able to see where it's going. I think that's tolerable, but maybe
the cursor keys should be disabled in that state as well.
TILE_GAP represents the gap between the coloured parts of adjacent
different-coloured tiles. Rather than a fixed 2 pixels, it's now 1/16
of the tilesize (rounded to nearest).
This looks the same at the default tilesize of 32, but behaves better
with larger or smaller tilesizes. At tilesizes below 8, the gap
disappears altogether, but that seems appropriate: at 7 pixels there's
not much space for the inner and outer squares and the cursor, and those
are all more important than the gap..
Flood's draw_tile() extravagantly uses up to eight rectangles to draw
separators around each tile. This could be substantially improved,
but a particularly low-hanging optimisation is not do draw them when
the separator width is zero.
This at least means that Flood completes its initial drawing on my
test KaiOS device.
I've rewritten tile_redraw to reduce the number of calls to
draw_rect(). Before, it would generally make five calls to
draw_rect() when drawing a tile. Now it makes at most three, and
usually two. That's one draw_rect() for each colour that appears in
the tile, which is as good as it can get. This reduces the time to
draw a large puzzle by about 35% on Firefox 102.
This is of significance to me because CanvasRenderingContext2D on my
test KaiOS device seems to have a limit on the number of fill() and
fillRect() calls that it will tolerate in a short time. This means
that if you issue more than 1024 fillRect() calls in rapid succession,
the later ones are simply ignored.
Same Game's largest preset called draw_rect() so much that it hit this
limit. That meant that the right-hand side of the grid didn't get
properly drawn when starting a new game. Now that it is less
profligate with draw_rect() it fits comfortably within the limit and I
get to see the entire grid.
The drawing routines in JavaScript used to take pointers to a C string
containing a CSS colour name. That meant that JavaScript had to create
a new JavaScript string on ever call to a drawing function, which seemed
ugly.
So now we instead pass colour numbers all the way down into JavaScript
and keep an array of JavaScript strings there that can be re-used. The
conversion from RGB triples to strings is still done in C, though.
This doesn't seem to have fixed either of the bugs I hoped it would, but
it does measurably improve drawing performance so I think it's worth
doing.
We were using free_cfg(be_prefs) after we copied be_prefs to
all_prefs, but we actually want to use sfree(be_prefs) since we don't
want to free each element that has been copied. None of the games so
far use string preferences, which is why they haven't been affected by
this bug.
The keyboard cursor in Same Game is white. The default yellow,
cyan, and light green were light enough to make the cursor hard to
see. I've darkened them all (without changing their hues) so that the
cursor is acceptably visible. This doesn't leave an ideal set of
colours, but they are at least still adequately distinct from one
another.
