[Commit message added by SGT: this makes it easier to allocate indices
in the config_item array, and keep them in sync between get_prefs and
set_prefs for each game.]
This refactors all instances of bitwise-ANDs with `~MOD_MASK'. There is
a handful of more complex instances I left unchanged (in cube.c, midend.c,
and twiddle.c), since those AND with `~MOD_MASK | MOD_NUM_KEYPAD' or
similar. I don't think it's worth writing a macro for those cases.
Also document this new macro's usage in devel.but.
When a drag started outside the grid (or no drag has started yet),
ensure the drag state in game_ui says so and bail out accordingly.
Previously, such a drag would manipulate the tile the last valid drag
started from, if any, else segfault.
Also, allow drags that start on-grid and then go off-grid to continue
rotating.
All the other constants named UI_* are special key names that can be
passed to midend_process_key(), but UI_UPDATE is a special return value
from the back-end interpret_move() function instead. This renaming
makes the distinction clear and provides a naming convention for future
special return values from interpret_move().
Net's loop highlighting detects any loop in the current state of the
grid. I've occasionally found that to be a bit of a spoiler, since
sometimes it can point out a deduction I should make before I've
figured it out for myself - e.g. when I've just locked all but two of
the squares involved in the loop, and the last two _just happen_ to be
oriented so as to complete the loop. In that situation I'd prefer if
the loop _didn't_ immediately light up and point out to me that I need
to arrange that those squares aren't connected to each other.
The simple answer is to only count edges connecting two _locked_
squares, for the purposes of loop detection. But this is obviously
unacceptable to some players - in particular, those who play without
the locking feature at all. So it should be a user preference.
These are similar to the existing pair configure() and custom_params()
in that get_prefs() returns an array of config_item describing a set
of dialog-box controls to present to the user, and set_prefs()
receives the same array with answers filled in and implements the
answers. But where configure() and custom_params() operate on a
game_params structure, the new pair operate on a game_ui, and are
intended to permit GUI configuration of all the settings I just moved
into that structure.
However, nothing actually _calls_ these routines yet. All I've done in
this commit is to add them to 'struct game' and implement them for the
functions that need them.
Also, config_item has new fields, permitting each config option to
define a machine-readable identifying keyword as well as the
user-facing description. For options of type C_CHOICES, each choice
also has a keyword. These keyword fields are only defined at all by
the new get_prefs() function - they're left uninitialised in existing
uses of the dialog system. The idea is to use them when writing out
the user's preferences into a configuration file on disk, although I
haven't actually done any of that work in this commit.
I'm about to move some of the bodgy getenv-based options so that they
become fields in game_ui. So these functions, which could previously
access those options directly via getenv, will now need to be given a
game_ui where they can look them up.
This is preparing to separate out the auxiliary functionality, and
perhaps leave space for making more of it in future.
The previous name 'edsf' was too vague: the 'e' stood for 'extended',
and didn't say anything about _how_ it was extended. It's now called a
'flip dsf', since it tracks whether elements in the same class are
flipped relative to each other. More importantly, clients that are
going to use the flip tracking must say so when they allocate the dsf.
And Keen's need to track the minimal element of an equivalence class
is going to become a non-default feature, so there needs to be a new
kind of dsf that specially tracks those, and Keen will have to call it.
While I'm here, I've renamed the three dsf creation functions so that
they start with 'dsf_' like all the rest of the dsf API.
In this commit, 'DSF' is simply a typedef for 'int', so that the new
declaration form 'DSF *' translates to the same type 'int *' that dsfs
have always had. So all we're doing here is mechanically changing type
declarations throughout the code.
The offset and centre location should be within the grid. Otherwise the
redraw code will suffer an assertion failure. This save file
demonstrates the problem:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :3:Net
PARAMS :4:5x5w
CPARAMS :4:5x5w
DESC :25:9893e85285bb72e6de5182741
UI :9:O0,0;C6,6
NSTATES :1:1
STATEPOS:1:1
Some games would like a way to check that the parameters in the encoded
UI string are consistent with the game parameters. Since this might
depend on the current state of the game (this being what changed_state()
is for), implement this by adding a game_state parameter to decode_ui().
Nothing currently uses it, though Guess usefully could.
This avoids an out-of-range heap write shortly afterwards. An assertion
failure is better than a buffer overrun, but still not ideal. Fixing
the problem properly will require fairly wide-ranging changes, though.
The bug can be demonstrated by loading this save file into a build with
AddressSanitizer:
SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :3:Net
PARAMS :4:5x5w
CPARAMS :4:5x5w
DESC :25:9893e85285bb72e6de5182741
UI :9:O0,0;C6,6
NSTATES :1:1
STATEPOS:1:1
This fixes a build failure introduced by commit 2e48ce132e011e8
yesterday.
When I saw that commit I expected the most likely problem would be in
the NestedVM build, which is currently the thing with the most most
out-of-date C implementation. And indeed the NestedVM toolchain
doesn't have <tgmath.h> - but much more surprisingly, our _Windows_
builds failed too, with a compile error inside <tgmath.h> itself!
I haven't looked closely into the problem yet. Our Windows builds are
done with clang, which comes with its own <tgmath.h> superseding the
standard Windows one. So you'd _hope_ that clang could make sense of
its own header! But perhaps the problem is that this is an unusual
compile mode and hasn't been tested.
My fix is to simply add a cmake check for <tgmath.h> - which doesn't
just check the file's existence, it actually tries compiling a file
that #includes it, so it will detect 'file exists but is mysteriously
broken' just as easily as 'not there at all'. So this makes the builds
start working again, precisely on Ben's theory of opportunistically
using <tgmath.h> where possible and falling back to <math.h>
otherwise.
It looks ugly, though! I'm half tempted to make a new header file
whose job is to include a standard set of system headers, just so that
that nasty #ifdef doesn't have to sit at the top of almost all the
source files. But for the moment this at least gets the build working
again.
C89 provided only double-precision mathematical functions (sin() etc),
and so despite using single-precision elsewhere, those are what Puzzles
has traditionally used. C99 introduced single-precision equivalents
(sinf() etc), and I hope it's been long enough that we can safely use
them. Maybe they'll even be faster.
Rather than directly use the single-precision functions, though, we use
the magic macros from <tgmath.h> that automatically choose the precision
of mathematical functions based on their arguments. This has the
advantage that we only need to change which header we include, and thus
that we can switch back again if some platform has trouble with the new
header.
If you define PUZZLES_INITIAL_CURSOR=y, puzzles that have a keyboard
cursor will default to making it visible rather than invisible at the
start of a new game. Behaviour is otherwise the same, so mouse actions
will cause the cursor to vanish and keyboard actions will cause it to
appear. It's just the default that has changed.
The purpose of this is for use on devices and platforms where the
primary or only means of interaction is keyboard-based. In those cases,
starting with the keyboard cursor invisible is weird and a bit
confusing.
For reasons now lost to history, Puzzles generally uses single-precision
floating point. However, C floating-point constants are by default
double-precision, and if they're then operated on along with a
single-precision variable the value of the variable gets promoted to
double precision, then the operation gets done, and then often the
result gets converted back to single precision again.
This is obviously silly, so I've used Clang's "-Wdouble-promotion" to
find instances of this and mark the constants as single-precision as
well. This is a bit awkward for PI, which ends up with a cast. Maybe
there should be a PIF, or maybe PI should just be single-precision.
This doesn't eliminate all warnings from -Wdouble-promotion. Some of
the others might merit fixing but adding explicit casts to double just
to shut the compiler up would be going too far, I feel.
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.
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.
Previously, the network, grid edges and barriers were all drawn with
fixed line thickness of one pixel, or three pixels in the case of
wires (one central one in a lit-up colour and a black border pixel on
each side). This worked badly on high-DPI displays, or in any other
environment where you expand the window noticeably.
I've tried a few times before to fix this, but the problem was always
that the Net drawing code was complicated and confusing, because Net
was one of the founding puzzles in this collection and its drawing
code predated a lot of the sensible organisation and best-practice
doctrines I've come up with since then and used in the later puzzles.
(The best example of this was the way that redrawing a grid square
always redrew _all_ the surrounding borders, which is very confusing
in the light of my later practice of dividing up the grid into
disjoint regions that are always redrawn with clipping.) It was hard
to make any change to the Net graphics with the code in that state; I
tried several times and decided I couldn't sensibly make it work
without throwing it all away and rewriting from scratch, which I was
always reluctant to do.
But not any more! Since Ian sent some patches to improve the graphics
scaling in Tracks, and since Net was at the top of my personal
wishlist of games that needed the same treatment, I've decided it's
time to do just that.
So, this commit throws out all of Net's previous redraw code, and
replaces it with something in the more modern style I usually adopt in
new puzzles. The new draw_tile() doesn't read data out of the game
state willy-nilly; instead it takes a single word of bit-flags
describing everything about the tile it's drawing, and makes all its
decisions based on that word. And the main redraw loop constructs a
whole array of flags words describing what it would _like_ the grid to
look like, and then calls draw_tile() for every square whose word
doesn't match the one that was previously drawn there. (With the one
exception that the presence of the TILE_ROTATING flag in either the
old or new word forces a redraw _even_ if the two words are identical,
because that's how the move animation works.)
The new graphics look more or less the same at the default resolution
(there may be a pixel difference here or there but I don't think it's
noticeable if so), but now they scale up properly if you enlarge or
maximise the puzzle window.
Net is one of the very oldest puzzles in the collection, and has
apparently been physically copying the complete collection of totally
immutable barrier data in every game state since 2004. About time it
stopped, I think!
The solver code still had an assumption, which must have dated before
the Solve menu option was introduced, that all puzzles presented to it
had at least one valid solution, and was enforcing that assumption by
assert(). Now the solver returns a more sensible failure code which
solve_game() can convert into a proper error message.
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.
Another oddity involving an empty square is that if it coincides with
the source square for highlights (either by original design of the
game id, or because the player Ctrl-moves the source square into an
empty grid cell during play), then everything stops being lit up as
active. That's fine - you can still play the game using other
indications of error, such as the loop detection highlight - but it
looks silly for the status line to say 'Active: 1/lots'. So in that
situation I suppress the 'active' counter completely; it comes back
when you move the source square to somewhere it's _possible_ to
highlight more than one square.
While I'm at it, I've also removed the active counter in the case
where the game is completely solved, because in that situation it's
more or less unnecessary anyway, and that way the normal course of
play on the default small grid size doesn't overflow the available
status line space.
A hand-typed grid is permitted to use the square type '0' (never
generated by Net's own grid generator), which is a completely empty
square. This requires an adjustment to the completion checker, so that
such squares aren't required to be connected; otherwise, a grid
containing one would be permanently uncompletable.
However, the completion checker missed one case - it was
unconditionally checking that all squares are connected to the _top
left corner_, on the basis that (before I thought of the zero square)
any source square is as good as any other if what you really want to
know is whether they're all connected to each other. But that means
that if the top left square _is_ the empty one, things to wrong - e.g.
5x5:02c328ade11adb129d7c3e524 would fail to give a completion flash.
Fixed by starting the completion-checking search from the first
non-empty square we find.
I've removed the old algorithm (the one described as 'footpath dsf' in
the findloop.c appendix comment, though I hadn't thought of that name
at the time), and replaced it with calls to the new API.
This should have no functional effect: there weren't any known bugs in
the previous loop-finder that affected currently supported play modes.
But this generality improvement means that non-orientable playing
surfaces could be supported in the future, which would have confused
the old algorithm. And Net, being the only puzzle as yet that's played
on a torus, is perhaps the one most likely to want to generalise
further at some point.
I unthinkingly transplanted into Net the same loop-finding algorithm
used in Loopy and Slant, which identifies the connected components
into which the grid lines divide the plane, and marks an edge as part
of a loop iff it separates two different components. This works fine
for a planar graph, but in Net's wrapping mode, it's possible to have
loops which do not have this property - e.g. a loop can go off the top
of the grid and back on the bottom to join up with itself, and then
it _doesn't_ disconnect the surface into two components.
(In principle, this kind of problem can turn up in any topological
space with a non-trivial H_1 homology group, which is why it fails on
the torus to which Net's wrapping mode corresponds, but not on the
plane or sphere. I think it's forgivable that I hadn't expected
homology to be the cause of any bug in practical code ever!)
Fixed by inventing yet another dsf-based loop-finding algorithm, this
one based on tracing round the outside of connected components of the
graph. It's still not _fully_ general, in that this one still depends
on the graph being drawn on an orientable surface (so it'll need
another rewrite if I ever add Mobius strip or Klein bottle modes for
Net...), but it's fairly simple to state and implement, and detects
loops that the previous implementation did not, such as the one in the
starting position of 3x3w:1a39ac6a8 .
Loops are detected using the same dsf technique I ended up using in
Slant, and highlighted in red (whether or not the connected component
they belong to is currently powered).
Should make life a little bit easier for someone who's filled in most
of the grid to a nice uniform cyan and finds one piece left over - now
they have some idea where to start looking for their mistake.
We also take care not to generate any loops in the starting position,
on grounds of politeness (don't accuse the user of a mistake before
they've even had a chance to make one).
Loop detection does not contribute to the code that decides whether
the puzzle is complete, because there's no need - if all squares are
connected together, then there can't be any loops anyway, by graph
theory.
puzzle backend function which ought to have it, and propagate those
consts through to per-puzzle subroutines as needed.
I've recently had to do that to a few specific parameters which were
being misused by particular puzzles (r9657, r9830), which suggests
that it's probably a good idea to do the whole lot pre-emptively
before the next such problem shows up.
[originally from svn r9832]
[r9657 == 3b250baa02a7332510685948bf17576c397b8ceb]
[r9830 == 0b93de904a98f119b1a95d3a53029f1ed4bfb9b3]
new_desc. Oddities in the 'make test' output brought to my attention
that a few puzzles have been modifying their input game_params for
various reasons; they shouldn't do that, because that's the
game_params held permanently by the midend and it will affect
subsequent game generations if they modify it. So now those arguments
are const, and all the games which previously modified their
game_params now take a copy and modify that instead.
[originally from svn r9830]
basically just so that it can divide mouse coordinates by the tile
size, but is definitely not expected to _write_ to it, and it hadn't
previously occurred to me that anyone might try. Therefore,
interpret_move() now gets a pointer to a _const_ game_drawstate
instead of a writable one.
All existing puzzles cope fine with this API change (as long as the
new const qualifier is also added to a couple of subfunctions to which
interpret_move delegates work), except for the just-committed Undead,
which somehow had ds->ascii and ui->ascii the wrong way round but is
otherwise unproblematic.
[originally from svn r9657]
midend_status(), and given it three return codes for win, (permanent)
loss and game-still-in-play. Depending on what the front end wants to
use it for, it may find any or all of these three states worth
distinguishing from each other.
(I suppose a further enhancement might be to add _non_-permanent loss
as a fourth distinct status, to describe situations in which you can't
play further without pressing Undo but doing so is not completely
pointless. That might reasonably include dead-end situations in Same
Game and Pegs, and blown-self-up situations in Mines and Inertia.
However, I haven't done this at present.)
[originally from svn r9179]
state is in a solved position, and a midend function wrapping it.
(Or, at least, a situation in which further play is pointless. The
point is, given that game state, would it be a good idea for a front
end that does that sort of thing to proactively provide the option to
start a fresh game?)
[originally from svn r9140]
the drawing API, for use by Loopy. It's optional: drawing.c will
construct an acceptable alternative using a filled polygon if the
front end doesn't provide it.
Net and Netslide previously had static functions called
draw_thick_line(), whose claim to the name is less justified and so
they've been renamed.
[originally from svn r8962]
