I noticed that state->common, which is shared between all the game
states in an undo chain, has a 'solved' flag in it. That's not right -
solvedness as a property of a particular state on the chain belongs in
the game_state itself, and having-at-one-point-been-solved-ness as a
persistent property of the whole chain belongs in the game_ui.
Fortunately, the game isn't actually doing it wrong:
state->common->solved is set once and then never read, so it must have
been left in from early abandoned code. Now removed.
latin_solver_diff_set takes a boolean input parameter to indicate
whether the 'Extreme'-level variant of set elimination is permitted.
But it's commented in the header file as having a boolean _output_
parameter which it sets if it _ended up_ using that variant. Probably
the latter was how it worked in an early draft, and I changed my mind
later without changing the comment.
In menu_save_event, we checked ctx.error to see if an errno value had
been left in it by the savefile_write callback, but if so, then we
were passing the _current_ value of errno to strerror() in place of
the saved value in ctx.error.
This may well have been benign, but I spotted it in an eyeball review
just now and thought I'd better fix it before it bit anyone.
To begin with, the toolchain no longer lets me build for x86-32 -
apparently MacOS is now 64-bit only.
Also, the linker now gives an error about a missing libgcc_s variant
for -mmacosx-version-min=10.4, and indeed 10.5. So I've bumped the
minimum supported OS version to 10.6.
That in turn required some changes in osx.m itself, because bumping
the min OS version caused some API deprecations to show up. Luckily I
turned out to have left myself a comment some time ago telling me what
I was going to need to do about one of them :-)
I've been playing this game a fair bit recently, and it's probably
time I jotted down some of the deductions I've been doing in my own
brain that the game doesn't know about. (Also I had an algorithmic
thought about the area-parity technique.)
For cube games, the minimum for any dimension should be 2, as there is
no net of the cube that is only one row/column. The previous logic
would permit a 1x7 game (for example) that could never be solved.
The read function used by midend_deserialise and friends is expected
never to perform a partial read (the main deserialisation code always
knows how many bytes it can expect to see), so it's specified to
return simply TRUE or FALSE for success/failure, rather than the
number of bytes read.
This probably wasn't breaking anything, since in the case of
deserialising from an internal memory buffer a short read could only
arise due to an outright bug constructing the buffer. But now I've
spotted it, I should fix it.
The JS code that retrieves the save-file data from emcc.c doesn't
receive a separate length value, but instead expects the data to be in
the form of a NUL-terminated string. But emcc.c wasn't NUL-terminating
it, so the save data could come out with random cruft on the end.
Instead of repeatedly reusing the file name 'PuzzleEngine.class' in
the main build directory, now each puzzle's NestedVM translation is
left in a separate subdirectory so that they don't collide with each
other. A bonus is that we don't have to rename the file back and forth
between the rule that builds it and the one that consumes it.
OS X is beginning to show a warning when a 32-bit application is
opened, so it's high time that this gets enabled. Fix a clang warning
exposed by this build.
This consists of setting a flag in the Info.plist. Everything in the
game is resolution-independent, of course, and has been thoroughly
tested on other platforms. The only issue I found is cosmetic: The
rounded window corners become more dramatic and don't look as good
against the square status bar.
Increasing the resolution also exposes two graphical quirks that I
don't think are new. Curved rails in Tracks seem to be made of short
segments that don't quite connect, but I don't see this on Android,
and on closer inspection this is already present on low resolution in
OS X. Lines in Untangle, and also Galaxies, that are at a multiple of
45 degrees seem thinner than other lines, but I also see this on
Android — I think it's just more obvious on high resolution, and could
be adjusted with antialiasing. Everything else looks as it should, for
example when moving a window between low and high dpi displays.
I've just upgraded my build machine to Ubuntu 18.04, which has come
with a version of javac that complains about both -source 1.3 and
-target 1.3. Both are surely pretty out of date anyway, so the path of
least resistance is to just increase them to the earliest version that
javac doesn't currently complain is deprecated.
If I want to rebuild just the Javascript puzzles (for example) in
circumstances where I don't expect to need a great many
edit-compile-link cycles, it's easier to get bob to do it for me than
to remember how to set up the development tools on my path. But it
takes ages to run the whole build script if I also have to wait for
the Windows, Mac and Java puzzles to be built, not to mention the
initial Unix build that runs for no purpose other than generating the
icon images.
So now I can run the build with various time-consuming parts
conditioned out, for development purposes. Of course, the default is
still to build absolutely everything.
Oops! This was left over from an early development version of commits
4408476b7 and 000ebc507, in which I initially arranged for each
adjacency list to be terminated with the sentinel value -1 instead of
separately storing an array of the lists' lengths.
I later changed the representation to make randomising the algorithm
easier (it's much easier to shuffle an array uniformly at random if
you _don't_ have to faff endlessly to work out its length). But this
write of a no-longer- needed sentinel value in the client code must
have survived the rewrite by mistake, and also somehow evaded all my
pre-commit testing with valgrind and asan.
A user reported that the Towers Javascript version was crashing on
startup, and I think this is the cause, because it seems to fix it for
me.
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.
