Checking with the standards, I think this is legal C99, but not legal
C89 - and we are compiling in C89 mode. Why _every_ version of gcc
didn't object, given all the warning and pedantry options, I'm not
sure, but one did, so I should fix it.
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.
Its ability to solve general network flow problems was never actually
used in this code base; it was _always_ used for the restricted
problem of finding a matching in an unweighted bipartite graph. So now
I've switched all its clients over to the new matching.c, maxflow is
no longer needed.
Tents needs to construct maximal matchings in two different
situations. One is the completion check during play, in which the
existence of a perfect matching between tents and trees is part of the
win condition; the other is the initial grid generation, in which we
find a _maximal_ matching between the tents we've already placed and
all the possible neighbouring squares that are candidates for the tree
positions. Both of those are switched over.
The new client code is a lot smaller and nicer, because we can throw
away the col[] and num[] permutations entirely.
Of course, this also means that every puzzle that incorporated latin.c
now has to link against matching.c instead of maxflow.c - but since I
centralised that secondary dependency into Recipe a few commits ago,
it's trivial to switch them all over at once.
This is a dedicated algorithm for finding a maximal matching in a
bipartite graph.
Previously I've been solving that problem in this code base using
maxflow.c, modelling the graph matching problem as a restricted case
of the optimal network flow problem and then using a full-strength
algorithm for the latter. That's overkill, and also algorithmically
wasteful - the H-K algorithm is asymptotically better, because it can
find multiple augmenting paths in each pass (hence getting the maximum
benefit out of each expensive breadth-first search), as a consequence
of not having to worry about lower- or higher-value augmenting paths
in this restricted problem.
So I expect this algorithm to be faster, at least in principle or in
large cases, when it takes over the jobs currently being done by
maxflow. But that's not the only benefit. Another is that the data
representation is better suited to the problems actually being solved,
which should simplify all the call sites; a third is that I've
incorporated randomisation of the generated matching into the H-K
module itself, which will further save effort at each call site
because they will no longer have to worry about permuting the
algorithm's input - they just have to pass it a random_state and it
will take care of that internally.
This commit only introduces the new matching.c and builds a test
utility for it. I haven't yet migrated any clients of maxflow.
It's silly to have every puzzle using latin.c separately specify in
its .R file the list of additional modules that latin.c depends on, or
for that matter to have them all have to separately know how to adjust
that for the STANDALONE_SOLVER mode of latin.c.
So I've centralised a new pair of definitions into the core Recipe
file, called LATIN and LATIN_SOLVER, and now a client of latin.c need
only ask for that to get all the necessary dependencies too.
Also, while I'm here, I've moved the non-puzzle-specific 'latincheck'
test program out of unequal.R into the central Recipe.
A user mailed me today having found it less than clear from the docs
that Galaxies will only accept a solution if the set of filled-in grid
edges consists of _exactly_ the ones that separate two distinct
regions, rather than consisting of _at least_ those and perhaps others
which neither break rotational symmetry or disconnect any region.
Firstly, it tries to use newstate after freeing it. I haven't come up
with any way of actually triggering that bug. I suppose you'd need a
grid that required recursion to solve, which the generator of course
does not normally create.
Secondly, it stores M_BLACK or M_WHITE in the grid before doing the
recursion, whereas it should store BLACK or WHITE. I believe that since
it tries M_BLACK first, which is zero, and since it's trying it on an
undecided square (also zero), this leads to infinite recursion, and an
eventual crash once you run out of stack space.
You can (sometimes) trigger this by asking for a hint on a grid where
you've already made a mistake.
e.g. on the puzzle desc below there's a "9" at (13,7). If you place a
black tile to its immediate left and right - (12,7) and (14,7) - then
press 'h', you get a beachball and then crash (on macOS at least - I
presume other OSes are similar).
15x15:i5g4d16a7a7c3b3b11f11_15d3p8b7_8b4h18c4d13b4i7a16k9a9i4b2d10c14h11b10_10b17p6d8_7f9b8b11c10a2a5d5g7i
Michael Quevillon points out that neither 2x1 nor 3x1 Solo can be made
into an X Sudoku puzzle, on the grounds that whatever number goes in
one corner of the grid is ruled out from both ends (and the centre, if
any) of the opposing diagonal, and hence the X constraint can't be
satisfied.
(Also fixed a spurious full stop on a neighbouring line.)
Spotted by Michael Quevillon. After checking the set of digits
appearing in one of the two main grid diagonals, we weren't clearing
the used[] array before using it to check the same thing about the
other diagonal. So if the first diagonal we check has one of
everything, then the second will be misidentified as correct (for the
purposes of game-completion detection, although not error
highlighting) even if it doesn't have one of everything.
The branch of the code that claimed the puzzle to be ambiguous was not
also re-running the solver with diagnostics enabled, so that if a user
tries to use this tool to hand-design a puzzle, they do not get
feedback on what the multiple legal solutions actually are.
In solver_show_working mode, we were logging all the deductions,
guesswork and backtracking, but not printing out the actual solution
(if any) reached at the end of each branch of the tree.
This is another situation in which the midend's state, at the time
midend_new_game tries to save it, is not such as to generate a viable
serialisation. In this case, it's because after the user enters a game
id into the Game > Specific or Game > Random Seed dialog box,
midend_set_config will have _already_ started overwriting important
fields of the midend such as me->desc and me->seed, before
midend_new_game is even entered.
In fact this caused an assertion failure (thanks to Lennard Sprong for
reporting it), because one of those fields was set to NULL, so the
resulting serialisation buffer was incomplete, leading to a
deserialisation error later. But I was lucky: if I hadn't been, the
serialisation might have been complete, and valid according to the
deserialisation code, but would have contained a mixture of the new
game's description and the old one's move chain, which would surely
have had far stranger results.
For the moment, I'm fixing this by simply not storing a serialisation
in that situation. Perhaps a nicer approach might be to store one
before starting to overwrite midend fields, and then have
midend_new_game swap it in if it's already been generated. That way
you _could_ still undo past an action like this. But preventing the
assertion failure is a good start.
Now, when we undo a New Game by deserialising the stored version of
the previous game, we start by serialising the _current_ game into a
second serialisation buffer in the midend. Then, if the user tries to
redo back past that undo action, we can re-serialise the earlier game
and re-deserialise the newer one.
A few users had complained (with various degrees of politeness) at the
lack of this ability, because in true xkcd #1172 style, it broke their
workflow. Specifically, if you were a fan of holding down 'u' to undo
all the way back to the start of your current game, you'd have
overshot into the previous game and then have no way to return to the
game you wanted to be at the start of.
This slightly changes the previous interaction of Redo with New Game.
Previously, New Game would save the entire undo chain of the
serialised game, including anything forward of the current position;
so if you took actions move1,move2,undo,newgame then the serialised
game state would include both move1 and move2 with the current
position between them; hence, an undo would restore the old game to a
position just after move1, and then a redo action would re-enact
move2. Now, midend_purge_states is called before serialising the old
game, so in that scenario move2 would be completely lost as a side
effect of the new-game action. (Just as it would be if you made any
_other_ undoable move in that situation.)
Conversely, making a move in the old game after you've undone back
into it will now wipe out the record of the later game, so you can't
redo into it any more.
The three related buf/len/size fields are now a sub-structure of the
main midend, and the serialise/deserialise functions that address
those fields now take a pointer to that sub-structure rather than to
the midend as a whole. This will make it easy for me to drop in a
second substructure alongside it, for redo, and reuse those read and
write functions by just changing the context pointer.
Editing LICENCE just now, I happened to notice that the accented
letter in Jonas Kölker's name was encoded in ISO 8859-1, as is the
occurrence of the same name in filling.c - but _not_ the one in
guess.c, which was in UTF-8 already. That seems needlessly confusing,
so let's sort it out. Now every text file in this git repository is
suitable for interpreting as UTF-8.
Regular hexagons and equilateral triangles in strict alternation, with
two of each interleaved around each vertex.
https://en.wikipedia.org/wiki/Trihexagonal_tiling
Thanks to Michael Quevillon for the patch.
There were a couple of places where we enforced by assertion that our
solver state had not become inconsistent, on the assumption that some
previous piece of solver would have detected that the puzzle was
impossible before getting to that place in the code.
But in fact the combination of Killer and Unreasonable modes falsified
at least two of those assumptions: 'solo --test-solve --generate 100
3x3kdu#12345' triggered an assertion failure in solver_set, and with
that one fixed, the same command triggered a second failure in
solver_killer_sums.
In both cases, the fix is simply to return -1 for 'puzzle is
inconsistent', which will cause the Unreasonable recursive solver to
abandon that branch of its search tree, backtrack, and try a different
guess at some previous square.
Thanks to Anders Höglund for the report.
The fields (ui->dragx,ui->dragy) stored the pixel coordinates of the
visible cursor (if any), no matter whether that cursor was being
displayed in response to a mouse dragging action or arrow-key
activity. But this meant that resizing the window while the keyboard
cursor was visible would cause the cursor to be drawn in totally the
wrong place in the newly resized window: you get a new drawstate with
a fresh blitter (so at least no part of the old-size window is
accidentally 'restored' on to the new-size one), but the ui fields
saying where _next_ to draw the cursor would still have bogus values
left over from the previous window size.
To fix this, I've arranged that we simply don't use ui->dragx and
ui->dragy any more in keyboard cursor mode: instead, we leave it to
game_redraw to spot that the keyboard cursor is visible and compute
its pixel coordinates for display.
A knock-on effect is that when we need to know which region is under
the cursor during interpret_move, we can't use the previous strategy
of just calling region_from_coords(ui->dragx, ui->dragy) regardless of
which kind of cursor is active. So I've split that function up into an
inner section taking a tile and a displacement from the tile's centre
and an outer part which derives those from real pixel coordinates in
the case where the cursor is originating from a mouse drag; then
there's a new alternative outer part which derives the same (tile,
displacement) pair purely from the game_ui keyboard cursor data
without having to explicitly translate into pixels and back in the
middle. Probably a less fragile strategy anyway.
I had left one mention of the new GTK3-only variable
'awaiting_resize_ack' unprotected by #ifdefs. Also, the GTK2 version
of message_box() was missing some consts in its prototype, presumably
because when I had that #ifdeffed out the compiler didn't warn me
about those ones.
A user sent in the game id
7:0,0L,0,0,0L,0L,1,0U,0U,0D,0UD,0,0,7,0,0D,0,0D,0,0,0,0,0,0,0,0,0,0D,0,0,3,0,0U,0,0,0,0,2,6,0,0U,0,0,0,7,2,0,0U,5L,
starting from which, one press of 'h' will fill in a 2 in the top left
corner and possibilities 345 to its right; if you then fill in 5 there
and press 'h' again, the hint system fills in the whole of an apparent
solution which isn't the one a proper use of the Solve feature would
give you - except that it's not actually a second solution, because
one < clue on the top row is violated. Without this fix, that < failed
to light up red, making the puzzle look ambiguous if you're not paying
enough attention to spot it.
prev[sink] == 0 means there was a path found with the last step being a
forward edge to the sink, and the edge being at index 0 in the array.
This is a valid path (the same as any other path indicated by prev[sink]
> 0).
The initial call to midend_new_game() was creating a partial
serialisation containing no game states at all, which meant that if
your first UI action was an undo operation, the game would try to
deserialise that and complain that it was incomplete. Now we detect
that in advance and don't create a useless serialisation in the first
place.
I went through all the char * parameters and return values I could see
in puzzles.h by eye and spotted ones that surely ought to have been
const all along.
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.
If I increase clang-cl's warning pickiness, it starts objecting to my
cast to HMENU from a (potentially, in 64 bits) smaller integer type.
Actually I don't think there's a problem there - all the integer ids I
cast to HMENU are nice small numbers and a widening cast is just fine.
But I can suppress the warning by using INT_PTR instead of int in the
prototype for mkctrl, so it's easiest just to do that.
The newgame_undo data was being saved on every call to
midend_new_game, including the one just after midend_set_params when a
new puzzle preset was selected. So you could select a new preset from
the menu, and then press 'u', and the midend would _try_ to undo that
operation and restore the previous game with a different set of
parameters.
This would do the wrong thing in the front end, because front ends in
general will not be expecting that a change of game parameters might
result from an arbitrary keyboard event - they won't be expecting to
have to call the function that moves the highlight in the game-type
menu, for example, and they _certainly_ won't be expecting that a
window resize might be necessary in response to a random keystroke.
One possible response would be to fix all the front ends so that they
_are_ prepared for either of those consequences of a keystroke event,
and then it would be possible to undo not only the New Game menu
option and the 'n' key but also undo any selection of a preset from
the game-type menu, or even a full re-customisation of the game
settings. But that would be quite an upheaval even in _my_ front end
collection, and also probably be awkward for downstream front ends, so
until I'm convinced of the value of going to all the effort, the
simpler approach is just to disallow undoing a new game in those
situations.
(This does mean that re-selecting the _already active_ game preset
from the type menu will be treated as an undoable new-game event,
which I think is an acceptable UI oddity.)
I've renamed the new midend variables to match my usual naming
convention of using 'size' for the total buffer size and 'len' for the
amount of currently used space (and a couple of other variables to
match those in turn), partly for consistency and also because the name
'midend_undo_used' made me half-expect it to be a boolean. The buffer
itself is called 'midend_undo_buf', again to avoid making it sound
like a function or flag.
Buffer growth is still geometric but less aggressive (factor of 5/4
each time instead of 2), in the hope of wasting less memory on low-RAM
platforms; newgame_undo_deserialise_read should have been static, and
now is; newgame_undo_buf is initialised using NULL rather than 0 so it
doesn't look like an integer, and is freed with the rest of the
midend.
And I think we _should_ enforce by assertion that midend_deserialise
didn't return an error, because there's no reason it ever should in
this situation (unlike when reading from a file, where the user might
have provided the wrong file or a corrupted one). This immediately
allowed me to spot a bug in the existing deserialisation code :-)
It is annoying when one intends to choose "restart" and chooses "new
game" instead. Right now, the puzzle one wanted to try again is
discarded.
To fix this we are going to have to save a lot more information than a
normal game state. Handily, we already have the serialise/deserialise
machinery.
The advantage of using this is that the previous game is easily saved
in its entirety, including its own undo history, and also probably in
a more compact format.
The (de)serialisation interface is rather clunky for use with a memory
target. Sadly none of the existing implementations of a resizing
memory array have been conveniently brought out into puzzles.h, and I
think that that's beyond the scope of what I wanted to do here.
We don't serialise the new game undo serialisation data. So
loading/saving doesn't preserve any "new game" undo, as does "new
game" twice (and as does context switching on a Palm Pilot).
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
This will let me do a 'conditional deserialisation' operation, in
which we fully decode the serialised data and then (assuming that gave
no errors) decide whether or not to actually install it based on some
arbitrary condition.
I don't think there's any possible use for the extra check function
_outside_ midend.c, so I've left the API for front ends as it is; the
externally visible midend_deserialise() doesn't have the new
parameter, and only midend_deserialise_internal() includes it.
Lots of the local variables in midend_deserialise are now fields of a
structure which contains everything that is _going_ to be written into
the midend once we finish validating it all. This makes it easy to
keep all that data together, and (in future) pass it to other
functions all in one go.
No functional change.
The serialised game stores a long-term and a short-term parameter
structure, which correspond to me->params (the thing that gets used by
the next New Game command) and me->curparams (the thing that _was_
used to generate _this_ game). So data relevant to the current game
ought to be validated against the latter, but in fact I was
accidentally passing the former to several validation calls.
I think this probably avoided causing a problem because typically
params and cparams don't differ very much: the usual reason why
they're not the same is that somebody has manually entered a game
description involving an incomplete description of the parameters
(lacking generation-specific details like difficulty level), but by
the very fact that those incomplete descriptions have to contain
_enough_ information to understand a specific game description,
copying just those parts of the description into the long-term params
structure makes the two similar enough that validation won't fail.
However, testing an upcoming patch which calls midend_deserialise at a
more difficult moment (specifically, just after midend_set_params,
meaning that the two params structures can now differ _arbitrarily_)
reveals my error. Fixed to use cparams where that's the right thing.
This makes it much easier to see the commonality in these formulaic
lines.
Whitespace change only.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
While working on the Net scalability today I noticed that changing
preset from (say) 13x11 to 5x5 in GTK3 Net while the window is
maximised does not have the desired effect (that being that, since the
maximised window does not change size, the new puzzle size is instead
scaled to fit neatly in the existing window).
A git bisect suggests that this was a side effect of commit 8dfe5cec3;
it looks as if there was a useful side effect of setting fe->area as
the 'geometry widget' for fe->window, namely, that any attempt to
resize the window thereafter (even if it had no effect on the window
size) would trigger a configure event on the geometry widget, so we'd
get a notification of our new size even if it was the same as our old
size.
But that 'geometry widget' feature is deprecated, so I have to work
around it another way. Fortunately, I've found a fallback event that
still does occur, namely "size_allocate" on fe->window. So I'm
trapping that as well and using it as an indication that a configure
event won't be forthcoming.
In commit a7dc17c42 I apparently introduced two bugs in
changed_preset(). Firstly, the Custom menu option was being written
into the 'found' variable in nearly all cases, because it has a NULL
user-data pointer which caused it to take the wrong branch of an if
statement due to an erroneous complex condition. Secondly, having
written _something_ into 'found', I had set it to inactive rather than
active due to forgetting to change a FALSE into a TRUE.
Now when I start up Net with my usual nonstandard default parameters
(I like the 13x11 wrapping, so I set NET_DEFAULT=13x11w in my
environment), the right menu entry comes up ticked.
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!
