This is essentially the same fix as 73c7bc090155ab8c was for Twiddle.
The new code is less clever but more correct (and more obviously
correct). The bug could be demonstrated by using a parameter string
of "c" or "m" with an AddressSanitizer build of Flood.
If execute_move() receieves a move that uses a colour beyond the range
for the current game, it now rejects it. Without this a solve string
containing an invalid colour would cause an assertion failure: "fill:
Assertion `oldcolour != newcolour' failed." While I was in the area I
put a range check on colours for normal moves as well. To demonstrate
the problem, load this save file:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :7:6x6c6m5
CPARAMS :7:6x6c6m3
DESC :39:432242034203340350204502505323231342,17
NSTATES :1:2
STATEPOS:1:2
MOVE :2:S6
To avoid assertion failures while painting it, we need to ensure that
the purported solution in a solve move doesn't include filling with the
current top-left colour at any point. That means checking the first
entry against the current top-left colours, and each later one against
its predecessor.
The fix in e4112b3 was incomplete: there was another assertion that could be failed by a save file with an ill-formed solve move. That now gets rejected properly. Here's an example save file to demonstrate the problem:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :5:Flood
PARAMS :7:6x6c6m0
CPARAMS :7:6x6c6m0
DESC :39:000000000000000000000000000000000000,00
NSTATES :1:2
STATEPOS:1:2
MOVE :1:S
If can_configure is false, then the game's configure() and
custom_params() functions will never be called. If can_solve is false,
solve() will never be called. If can_format_as_text_ever is false,
can_format_as_text_now() and text_format() will never be called. If
can_print is false, print_size() and print() will never be called. If
is_timed is false, timing_state() will never be called.
In each case, almost all puzzles provided a function nonetheless. I
think this is because in Puzzles' early history there was no "game"
structure, so the functions had to be present for linking to work. But
now that everything indirects through the "game" structure, unused
functions can be left unimplemented and the corresponding pointers set
to NULL.
So now where the flags mentioned above are false, the corresponding
functions are omitted and the function pointers in the "game" structures
are NULL.
A solve move containing characters other than digits and commas would
cause an assertion failure, "*p == ','", in execute_move(). Such a move
can't as far as I know be generated in play, but can be read from a
corrupt save file.
Here's a sample of such a save file:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :7:3x3c6m5
CPARAMS :7:3x3c6m5
DESC :12:403011503,10
NSTATES :1:2
STATEPOS:1:2
SOLVE :2:SA
This avoids an assertion failure, "oldcolour != newcolour" in fill(),
by catching it it execute_move(). As far as I know this couldn't be
triggered from the UI, but it could be demonstrated with this save
file:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :1:3
CPARAMS :1:3
DESC :12:231353400,11
NSTATES :1:3
STATEPOS:1:3
MOVE :2:M3
MOVE :2:M3
This provides a way for the front end to ask how a particular key should
be labelled right now (specifically, for a given game_state and
game_ui). This is useful on feature phones where it's conventional to
put a small caption above each soft key indicating what it currently
does.
The function currently provides labels only for CURSOR_SELECT and
CURSOR_SELECT2. This is because these are the only keys that need
labelling on KaiOS.
The concept of labelling keys also turns up in the request_keys() call,
but there are quite a few differences. The labels returned by
current_key_label() are dynamic and likely to vary with each move, while
the labels provided by request_keys() are constant for a given
game_params. Also, the keys returned by request_keys() don't generally
include CURSOR_SELECT and CURSOR_SELECT2, because those aren't necessary
on platforms with pointing devices. It might be possible to provide a
unified API covering both of this, but I think it would be quite
difficult to work with.
Where a key is to be unlabelled, current_key_label() is expected to
return an empty string. This leaves open the possibility of NULL
indicating a fallback to button2label or the label specified by
request_keys() in the future.
It's tempting to try to implement current_key_label() by calling
interpret_move() and parsing its output. This doesn't work for two
reasons. One is that interpret_move() is entitled to modify the
game_ui, and there isn't really a practical way to back those changes
out. The other is that the information returned by interpret_move()
isn't sufficient to generate a label. For instance, in many puzzles it
generates moves that toggle the state of a square, but we want the label
to reflect which state the square will be toggled to. The result is
that I've generally ended up pulling bits of code from interpret_move()
and execute_move() together to implement current_key_label().
Alongside the back-end function, there's a midend_current_key_label()
that's a thin wrapper around the back-end function. It just adds an
assertion about which key's being requested and a default null
implementation so that back-ends can avoid defining the function if it
will do nothing useful.
I don't know how I've never thought of this before! Pretty much every
game in this collection has to have a mechanism for noticing when
game_redraw is called for the first time on a new drawstate, and if
so, start by covering the whole window with a filled rectangle of the
background colour. This is a pain for implementers, and also awkward
because the drawstate often has to _work out_ its own pixel size (or
else remember it from when its size method was called).
The backends all do that so that the frontends don't have to guarantee
anything about the initial window contents. But that's a silly
tradeoff to begin with (there are way more backends than frontends, so
this _adds_ work rather than saving it), and also, in this code base
there's a standard way to handle things you don't want to have to do
in every backend _or_ every frontend: do them just once in the midend!
So now that rectangle-drawing operation happens in midend_redraw, and
I've been able to remove it from almost every puzzle. (A couple of
puzzles have other approaches: Slant didn't have a rectangle-draw
because it handles even the game borders using its per-tile redraw
function, and Untangle clears the whole window on every redraw
_anyway_ because it would just be too confusing not to.)
In some cases I've also been able to remove the 'started' flag from
the drawstate. But in many cases that has to stay because it also
triggers drawing of static display furniture other than the
background.
The Rockbox frontend allows games to be displayed in a "zoomed-in"
state targets with small displays. Currently we use a modal interface
-- a "viewing" mode in which the cursor keys are used to pan around
the rendered bitmap; and an "interaction" mode that actually sends
keys to the game.
This commit adds a midend_get_cursor_location() function to allow the
frontend to retrieve the backend's cursor location or other "region of
interest" -- such as the player location in Cube or Inertia.
With this information, the Rockbox frontend can now intelligently
follow the cursor around in the zoomed-in state, eliminating the need
for a modal interface.
This is the main bulk of this boolification work, but although it's
making the largest actual change, it should also be the least
disruptive to anyone interacting with this code base downstream of me,
because it doesn't modify any interface between modules: all the
inter-module APIs were updated one by one in the previous commits.
This just cleans up the code within each individual source file to use
bool in place of int where I think that makes things clearer.
This commit removes the old #defines of TRUE and FALSE from puzzles.h,
and does a mechanical search-and-replace throughout the code to
replace them with the C99 standard lowercase spellings.
encode_params, validate_params and new_desc now take a bool parameter;
fetch_preset, can_format_as_text_now and timing_state all return bool;
and the data fields is_timed, wants_statusbar and can_* are all bool.
All of those were previously typed as int, but semantically boolean.
This commit changes the API declarations in puzzles.h, updates all the
games to match (including the unfinisheds), and updates the developer
docs as well.
This function gives the front end a way to find out what keys the back
end requires; and as such it is mostly useful for ports without a
keyboard. It is based on changes originally found in Chris Boyle's
Android port, though some modifications were needed to make it more
flexible.
This allows me to use different types for the mutable, dynamically
allocated string value in a C_STRING control and the fixed constant
list of option names in a C_CHOICES.
Now midend.c directly tests the returned pointer for equality to this
value, instead of checking whether it's the empty string.
A minor effect of this is that games may now return a dynamically
allocated empty string from interpret_move() and treat it as just
another legal move description. But I don't expect anyone to be
perverse enough to actually do that! The main purpose is that it
avoids returning a string literal from a function whose return type is
a pointer to _non-const_ char, i.e. we are now one step closer to
being able to make this code base clean under -Wwrite-strings.
To do this, I've completely replaced the API between mid-end and front
end, so any downstream front end maintainers will have to do some
rewriting of their own (sorry). I've done the necessary work in all
five of the front ends I keep in-tree here - Windows, GTK, OS X,
Javascript/Emscripten, and Java/NestedVM - and I've done it in various
different styles (as each front end found most convenient), so that
should provide a variety of sample code to show downstreams how, if
they should need it.
I've left in the old puzzle back-end API function to return a flat
list of presets, so for the moment, all the puzzle backends are
unchanged apart from an extra null pointer appearing in their
top-level game structure. In a future commit I'll actually use the new
feature in a puzzle; perhaps in the further future it might make sense
to migrate all the puzzles to the new API and stop providing back ends
with two alternative ways of doing things, but this seemed like enough
upheaval for one day.
The only situation in which it actually can't find a solution is if
the puzzle is already solved, in which case it can at least fill in
*error to say so before it returns NULL.
Aapo Rantalainen points out that comparing 'char c' against zero gives
rise to gcc's "comparison is always false" warning, which escalates to
an error under -Werror.
This is one of those situations where the warning is doing more harm
than good, but here's a rephrasing which casts to unsigned so that
both negative numbers and positive out-of-range ones can be caught by
the same comparison.
Previously it simply chose every move based on the static evaluation
function 'minimise the pair (longest shortest-path to any square,
number of squares at that distance)'. Now it looks three moves ahead
recursively, and I've also adjusted the evaluation function to tie-
break based on the number of squares brought to distance zero (i.e.
actually in control).
The result isn't an unconditional improvement on the old solver; in a
test run based on 'flood --generate 1000 12x12c6m0#12345' I found that
57 out of 1000 grids tested now had longer solutions. However, about
three quarters had shorter ones, and solutions are more than a move
shorter on average.
The ones I started with were a bit under-varied and over-orthogonal.
Get rid of some of the more pointless things like 16x16 with lots of
extra moves, and add some with different colour counts. While I'm
here, make the menu descriptions nicer.
Based on a web game I saw a few years ago, and dashed off this weekend
after I thought of a way to write a good (though not quite optimal)
heuristic solver, here's a random little thing not quite in the same
line as the most usual kind of Puzzles fare: instead of making you
scratch your head to find any move to make at all, it's easy to find
solutions in principle, and the challenge comes from having to do so
within a move limit.
