The default one is always the same, because the main purpose of this
tool is debugging. But one person has already wanted to use it for
actually generating a tiling patch for another use, so let's make it
easier to vary the randomness!
This fills in the missing piece of commit 6f75879e9fe7cb5, which was
trying to make the output patches of tiling as uniformly random as
possible across the whole space of possible ones. I fixed every
_intermediate_ step in the algorithm, but forgot the starting one!
The new Hats tiling generates a lot of clues that are 2-digit numbers.
At large puzzle sizes, the previous clip rectangle didn't quite
include the ends of such a number, meaning that if the number had to
be redrawn in red to highlight an error, the leftmost and rightmost
parts of the text would remain black.
By introducing a second callback between the client of hat.c and the
maybe_report_hat function, I enable the test main() to provide a
different version of that callback, so that instead of enumerating
each kite, it can directly generate a Postscript path per actual hat.
This should make it more useful to people wanting to generate hat
patterns for any other purpose.
The internal callback gets more details than the external one; in
particular, it receives a HatCoords, so that it can colour hats based
on their position in the hierarchical structure.
This tweak improves the uniformity of the generated patches of hat
tiling, by selecting from (the closest 32-bit approximation I can get
to) the limiting probability distribution of finite patches in the
whole plane.
This shouldn't invalidate any grid description that contains enough
coordinates to uniquely specify a piece of tiling - in particular, any
generated by the game itself. But if anyone's been brave enough to
hand-type a grid description in the last two days and left off some of
the coordinates, then those might be invalidated.
I renamed it in a hurry this morning after the first report of a git
error message on Windows. Now I realise that several source files
referred to the old name, and also need fixing.
The big mathematical news this month is that a polygon has been
discovered that will tile the plane but only aperiodically. Penrose
tiles achieve this with two tile types; it's been an open question for
decades whether you could do it with only one tile. Now someone has
announced the discovery of such a thing, so _obviously_ this
mathematically exciting tiling ought to be one of the Loopy grid
options!
The polygon, named a 'hat' by its discoverers, consists of the union
of eight cells of the 'Kites' periodic tiling that Loopy already
implements. So all the vertex coordinates of the whole tiling are
vertices of the Kites grid, which makes handling the coordinates in an
exact manner a lot easier than Penrose tilings.
What's _harder_ than Penrose tilings is that, although this tiling can
be generated by a vaguely similar system of recursive expansion, the
expansion is geometrically distorting, which means you can't easily
figure out which tiles can be discarded early to save CPU. Instead
I've come up with a completely different system for generating a patch
of tiling, by using a hierarchical coordinate system to track a
location within many levels of the expansion process without ever
simulating the process as a whole. I'm really quite pleased with that
technique, and am tempted to try switching the Penrose generator over
to it too - except that we'd have to keep the old generator around to
stop old game ids being invalidated, and also, I think it would be
slightly trickier without an underlying fixed grid and without
overlaps in the tile expansion system.
However, before coming up with that, I got most of the way through
implementing the more obvious system of actually doing the expansions.
The result worked, but was very slow (because I changed approach
rather than try to implement tree-pruning under distortion). But the
code was reusable for two other useful purposes: it generated the
lookup tables needed for the production code, and it also generated a
lot of useful diagrams. So I've committed it anyway as a supporting
program, in a new 'aux' source subdirectory, and in aux/doc is a
writeup of the coordinate system's concepts, with all those diagrams.
(That's the kind of thing I'd normally put in a huge comment at the
top of the file, but doing all those diagrams in ASCII art would be
beyond miserable.)
From a gameplay perspective: the hat polygon has 13 edges, but one of
them has a vertex of the Kites tiling in the middle, and sometimes two
other tile boundaries meet at that vertex. I've chosen to represent
every hat as having degree 14 for Loopy purposes, because if you only
included that extra vertex when it was needed, then people would be
forever having to check whether this was a 13-hat or a 14-hat and it
would be nightmarish to play.
Even so, there's a lot of clicking involved to turn all those fiddly
individual edges on or off. This grid is noticeably nicer to play in
'autofollow' mode, by setting LOOPY_AUTOFOLLOW in the environment to
either 'fixed' or 'adaptive'. I'm tempted to make 'fixed' the default,
except that I think it would confuse players of ordinary square Loopy!
Now that we catch JavaScript errors and report them to the user, I could
tell that the way we were detecting the presence of getKaiAd() didn't
work because it caused an alert every time a build without KaiAds was
started. Detecting the presence of a global variable is slightly
tricky, but explicitly looking for it on "window" works and isn't very
ugly.
Most KaiOS devices are primarily keyboard-based, so this seems like a
reasonable approach.
I've also switched to specifying boolean values as JSON booleans because
that works now.
So a value is true iff it begins with 'T', 't', 'Y', or 'y'. This is
mostly so that naively converting JSON "true" to a string will work
properly, but it should keep LISPers happy too.
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.
This provides a standard way to get a boolean from an environment
variable. It treats the variable as true iff its value begins with 'y'
or 'Y', like most of the current implementations. The function takes a
default value which it returns if the environment variable is undefined.
This replaces the various ad-hoc tests of environment variable scattered
around and mostly doesn't change their behaviour. The exceptions are
TOWERS_2D in Towers and DEBUG_PUZZLES in the Windows front end. Both of
those were treated as true if they were defined at all, but now follow
the same rules as other boolean environment variables.
Ben complained yesterday that Galaxies had a nasty habit of generating
games whose solution was a boring set of rectangles. Now that even at
Normal mode the solver is better at coping with wiggly tentacled
regions, it seems like a good moment to fix that.
This change arranges that when we initially generate a filled grid, we
try ten times, and pick the wiggliest of the grids we found. This
doesn't make any boring rectangle-filled grid _impossible_ - players
will still have to stay on their toes, and can't rely 100% on at least
a certain number of wiggles existing - but it makes the interesting
grids a lot more likely to come up.
This skew happens before checking solubility. So it doesn't increase
grid generation time by a factor of ten (as it would if we generated
ten _soluble_ grids and picked the wiggliest). It's still a speed
drop, of course, but a more modest one than that.
It's horrible to have static mutable state in the live puzzle game.
What if some downstream wanted to run the system in multiple threads?
For purposes of limiting the recursion depth, we now pass an 'int depth'
argument to each call to solver_state_inner(). So in the normal build
of the actual puzzle, the static variable isn't needed at all. We only
include it in binaries that are going to want to use it for printing
indented diagnostics: the CLI solver program, and the live puzzle only
if DEBUGGING is defined. The rest of the time, it's absent.
A side effect of this change is that when the recursion code makes a
guess at a particular tile, the message about that is now indented to
the _outer_ level instead of the inner one, because the previous
'depth++' and 'depth--' statements wrapped the whole loop rather than
just the recursive call to the solver inside. This makes recursive
solving much easier to follow!
Most games in this collection don't have one. If you ask them for a
hard puzzle, they'll just keep looping round until they actually
manage to deliver one. We try to arrange that the standard presets
don't take too long to generate, but if the user turns up the game
size _and_ uses an expensive difficulty level, our view is that that's
up to them.
After fixing the bug from the previous commit in which the Galaxies
recursion depth limit was not actually doing anything, even 15x15
Unreasonable now generates happily in under a second. So I don't see
any reason why Galaxies should be an exception.
Hence, now if you ask Galaxies for an Unreasonable puzzle, you _will_
get a puzzle that it grades as Unreasonable, even if that takes a long
time to generate.
The static variable 'solver_recurse_depth' is _mostly_ used by the
standalone solver, to appropriately indent the solver diagnostics for
the current recursion level. So most uses of it are guarded by an
'#ifdef STANDALONE_SOLVER' statement, or some equivalent (such as
being inside the solvep() macro).
One exception is the check that limits the recursion depth to 5, to
avoid getting hung up forever on a too-hard game. Unfortunately, this
check depends on the variable actually incrementing when we recurse
another level - and it wasn't, because the increment itself was under
ifdef! So the generator in live Galaxies could recurse arbitrarily
deep, and generate puzzles that the standalone solver found too hard
_even_ at Unreasonable mode.
Removed the ifdefs, so that solver_recurse_depth is now incremented
and decremented. Also, make sure to initialise the depth to 0 at the
start of a solver run, just in case it had a bogus value left over
from a previous run.
I've often noticed that Galaxies on 7x7 Unreasonable often generates
puzzles that _I_ don't feel as if I had to use recursion and
backtracking to solve, suggesting that there's an efficient mode of
reasoning available that the puzzle could be using and isn't.
One reason for this is that sometimes Galaxies gives up on trying to
generate an Unreasonable puzzle (if its MAXTRIES counter runs out) and
knowingly falls back to Normal. But that's not the only reason. The
other day I got the puzzle 7x7:gsjgzhfedwgzhd, which Galaxies's own
solver rates as Unreasonable, and I still think it should be Normal.
The full solution to that puzzle is this:
+-+-+-+-+-+-+-+
|y x| o | | |
+ +-+-+-+-+ +o+
| | | | o | |
+ + o + + +-+-+
| | | | | | |
+ +-+-+ +-+o+ +
| o |o| |o|
+ +-+-+ +-+-+ +
| | | | o | |
+ + o + +-+-+-+
| | | | |
+-+-+-+ + +o+ +
| o |x y| |
+-+-+-+-+-+-+-+
and Galaxies's Normal-mode solver gets stuck on it at the point where
it's managed to deduce that the tiles labelled 'x' can't possibly be
associated with any dot other than leftmost dot on the centre row, but
is then unable to figure out that the tiles labelled 'y' must also be
associated with that dot. But clearly they must, because they're boxed
in on all other sides (by the grid edge, and by tiles whose
associations are totally obvious), so if either 'x' tile is to find
_any_ path back to its home dot, it must go through the neighbouring
'y' tile.
So, this commit adds the missing deduction: we use a dsf to identify
'exclaves' (connected sets of tiles all associated to the same dot,
but which do not actually contain the dot), and for each exclave we
count its 'liberties' (unassociated tiles bordering the exclave, i.e.
which would extend the exclave if associated to the same dot). Any
exclave with only one liberty must extend into that tile, or else it
would be cut off completely from its home dot. In this case, each 'x'
tile is an exclave by itself, with 'y' its only liberty. (And once
that deduction is done, the pair {x,y} become a larger exclave, which
can be deduced in the same way to connect to the next tile.)
I think this is a deduction rule simple and obvious enough that it
should go in at Normal mode. I've been using it all along in my own
play, and was surprised to find the game wasn't already taking it into
account. In addition, in a quick cross-test of the two versions,
_most_ 7x7 Normal games generated by the modified Galaxies are still
rated as Normal by the old less powerful solver. So it doesn't extend
the difficulty of Normal mode by very much, if at all.
Another benefit is that this should make Normal puzzles more likely to
contain twisty regions of this type.
Also, of course, the usual effect of adding extra deductions at levels
below Unreasonable means that actually Unreasonable puzzles become
that much more tricky!
I have a couple of ideas for extending this technique to be more
powerful still (filled in as comments at the top of the file). For the
moment, I've just done the most obvious version. Perhaps the others
might need to go in at a higher difficulty level.
The coordinates in this 'Setting edge' message from the solver don't
match the coordinates of the edge that is actually set. No wonder the
solver output was confusing!
In all front ends other than JavaScript, and in JavaScript prior to my
recent changes, MOD_NUM_KEYPAD can only be set on ASCII digits. For
now, enforce this in midend_process_key() so as to avoid inconsistency
between front ends. It might be useful to be able to distinguish
keypad versions of more keys, but that should be co-ordinated across
the code-base.
The two Return/Enter keys have always been treated the same in the
past, but a user complained today that Enter was no longer functioning
as CURSOR_SELECT in the web puzzles.
This happened in commit 9dbcfa765ba59a8, apparently because the web
front end is now translating the Enter key as MOD_NUM_KEYPAD | '\r'
instead of just '\r', and the new code in midend.c is only stripping
off MOD_NUM_KEYPAD for values >= 0x80.
Now it strips MOD_NUM_KEYPAD off C0 control characters as well, so
that only _printable_ ASCII characters can still have that modifier
when they get to the backend - i.e. you can tell numpad digits from
normal digits, and ditto +-* etc. But keypad Enter is now turned into
plain '\r' by the modifier removal code, and then into CURSOR_SELECT.
Other front ends still aren't even bothering to set MOD_NUM_KEYPAD on
the code sent by Enter. But that's fine, because now midend.c
officially doesn't care whether they do or not.
Just checking for the getKaiAd() function by doing "if (!getKaiAd ..."
throws a ReferenceError if it's not defined, and now my error box
catches that. Using "if (getKaiAd === undefined ..." seems to be
acceptable, though.
Element.setPointerCapture() captures both pointer and mouse events, so
we can use our existing mouse handlers and just have a minimal
pointerdown handler that calls setPointerCapture(). This saves (or at
least postpones) all the tedious rewriting referred to in ecd868ac.
This means that now drags beyond the puzzle area work in WebKit-based
browsers, and we don't get deprecation warnings in current Gecko-based
ones. Older Gecko-based browsers continue to use Element.setCapture()
and hence still work correctly.
Any solution actually generated by the solver will contain at least one
move, because it refuses to solve games that are already solved.
However, a save file might contain an empty "solve" move. This causes
an uninitialised read when execute_move() then tries to check if the
next move is in accordance with the solution, because the check for
running off the end of the solution happens after that.
We now avoid this by treating a zero-length "solution" as an invalid
move.
A save file that ended in the middle of a value before the "SAVEFILE"
field had been loaded would cause a read from uninitialised memory.
While technically undefined behaviour this was practically pretty
harmless. Fixed by handling unexpected EOF here the same an
unexpected EOF anywhere else.
This bug could be demonstrated by loading a truncated save file like
this in a build with MemorySanitizer enabled:
SAVEFILE:41:Simo
On a 32-bit system, evaluating 1<<31 causes undefined behaviour because
1 is signed and so it produces signed overflow. UBSan has spotted a
couple of occasions where this happens in Puzzles, so in each case I've
converted the left operand to the unsigned result type we actually want.
It now strips off modifier flags from keys that shouldn't have them and
maps printable characters with MOD_CTRL to the corresponding control
characters. It also catches Ctrl+Shift+Z because that obviously belongs
in the midend.
I've updated the JavaScript front-end to take advantage of these
changes. Other front ends are unchanged and should work just as they
did before.
I've changed my mind already. The other versions of fuzzpuzz all have
different command-line interfaces anyway, so I think the best approach
is to just accept that and decide that precisely how fuzzpuzz works
isn't a defined API. Fuzzing is inherently not an end-user activity, so
I think it's acceptable to make it a bit inconsistent.
This means that in Clang builds you get the non-libFuzzer version of
fuzzpuzz by default (so you can use it with other fuzzers), but if you
turn on WITH_LIBFUZZER then you'll get the libFuzzer version instead.
I've separated out the various versions of main(), which has helped a
little bit. I've also stopped using fmemopen() since libFuzzer might
work on Windows. But I think I probably still have something
fundamentally wrong in my approach.
For AFL++ and Honggfuzz, our approach is to build a standard fuzzpuzz
binary with extra hooks for interacting with an external fuzzer. This
works well for AFL++ and tolerably for Honggfuzz. LibFuzzer, though,
provides its own main() so that the resulting program has a very
different command-line interface from the normal one. Also, since
libFuzzer is a standard part of Clang, we can't decide whether to use it
based on the behaviour of the compiler.
So what I've done, at least for now, is to have CMake detect when we're
using Clang and in that case build a separate binary called
"fuzzpuzz-libfuzzer" which is built with -fsanitize=fuzzer, while the
ordinary fuzzpuzz is built without. I'm not sure if this is the right
approach, though.
This reverts commit 9d7c2b8c83506c1f239c840e372058fac603b255.
I thought that switching from the JS 'mousedown', 'mousemove' and
'mouseup' events to the corresponding 'pointer*' events would make
essentially no difference except that the pointer events would come
with more information. But in fact it turns out that there's a
fundamental change of semantics.
If you press one mouse button down and then, without releasing it,
press a second one, then the mouse API will send you this information
in the form of two 'mousedown' events, one for each button. But the
pointer API will only send you a 'pointerdown' for the first event,
when the state of the pointer changes from 'no buttons down' to 'at
least one button down'. The second button press will be delivered as a
'pointermove', in which the 'buttons' field is different from its
previous value.
I'm backing out the migration to PointerEvent for the moment, because
that's too complicated for a trivial fix. In simple cases we could
easily detect the changed buttons field in the pointermove handler and
generate a call to the C side of this front end's mousedown()
function, effectively converting the changed JS representation to the
one the C was already expecting. But this also has to interact with
our one-button support (converting Ctrl and Shift clicks into a
different logical button) _and_ with the ad-hoc mechanism we use to
avoid delivering buttonless mouse movements to the C side. So getting
it right in all cases at once isn't trivial, and I'd rather revert the
attempt now and think about it later than commit to getting it all
perfect on short notice.
After commit 1470c9530b1cff3 enabled assertions, I found that my build
scripts were complaining that the Windows binaries built from the same
source twice were not generating the same output, and it turned out to
be because the use of __FILE__ in every assert was baking in a
pathname from my build setup containing a mkstemp()-randomised path
component.
I've found the '-fmacro-prefix-map' option, available in both gcc and
clang (except the archaic gcc used by my NestedVM build, alas), which
lets you remap pathnames for purpose of what __FILE__ expands to. So
now our assertion statements should look as if the puzzle source just
lived in /puzzles, and (just in case a pathname in a generated header
ever becomes relevant) the cmake build directory is /build.
If the browser knows what 'PointerEvent' means, then we switch our
'onmousefoo' event handlers to the 'onpointerfoo' events, for both the
puzzle canvas and the resize handle.
The immediate effect of this is that we get to use the
setPointerCapture method on the puzzle canvas, in preference to the
deprecated Firefox-only setCapture.
A pointer event also contains extra fields compared to a mouse event:
as well as telling you which pointing device the event comes from, it
can also provide extra information, such as pressure, or the angle of
a stylus if the hardware can detect it. I don't have any immediate
ideas about what those could be used for, but it can't hurt to have
them available just in case we think of something in future.
element.setCapture only seems to exist in Firefox. On most other
browsers, our attempt to call it must have been generating a whinge in
the console log all along. But Ben's commit bb16b5a70ddf77d turned
that into a prominent alert box, triggered on every mouse click in the
puzzle canvas.
Worked around by wrapping both calls to setCapture in a local
subroutine which checks if it's there before calling it.
Also, setCapture turns out to be deprecated in any case, according to
https://developer.mozilla.org/en-US/docs/Web/API/Element/setCapture .
It looks as if the non-deprecated version is element.setPointerCapture:
https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture
But it also looks as if that needs the 'pointerId' field that's only
found in 'onpointerdown' events and not 'onmousedown' ones. So
including that as an alternative will be a bigger job.
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.
Despite the name, COMPILE_DEFINITIONS was only ever used to set a
single definition, and as far as I can tell that's all it could do
even when I tried to put them in a single word separated by
semicolons. Turning COMPILE_DEFINITIONS into a multi-valued argument
seems to make it work much better.
Assertion failures are ugly, but they're better than the alternative.
Defensive coding is a general principle throughout Puzzles and I don't
think it's sensible to selectively turn that off.
The mechanism by which we re-enable assertions is stolen from PuTTY
(with an enhancement to cover MinSizeRel builds as well) and is pretty
ugly because CMake doesn't seem to have a good way to do it.
I'm not quite sure how useful it will be, but it does at least catch
an assertion failure in main() and present an opaque message in a box,
which is better than stopping and putting a message in the console
where no-one will see it.
