When we set up a configuration sheet, we track the minimum overall
width that the controls will fit into (in a variable 'totalw'), and
separately, the minimum width needed by each of the left and right
columns containing control labels and actual controls ('leftw' and
'rightw'). If totalw > leftw+rightw at the end of the process, then we
must expand the two columns so that they have the right sum.
However, sometimes leftw+rightw can be zero, while totalw > 0. This
occurs if _no_ control in the box was of a type that used the left and
right columns for different things, so that the entire loop over the
controls only incremented totalw, and not leftw or rightw. For
example, in a puzzle such as Cube that defines no preferences of its
own, the only control in the preferences pane is midend.c's standard
"Keyboard shortcuts without Ctrl" preference, which is C_BOOLEAN and
only uses totalw.
In that situation, the code for proportionate distribution of the
excess divides by zero. So it needs a special case.
The new generator works on the same 'combinatorial coordinates' system
as the more recently written Hats and Spectre generators.
When I came up with that technique for the Hats tiling, I was already
tempted to rewrite the Penrose generator on the same principle,
because it has a lot of advantages over the previous technique of
picking a randomly selected patch out of the subdivision of a huge
starting tile. It generates the exact limiting distribution of
possible tiling patches rather than an approximation based on how big
a tile you subdivided; it doesn't use any overly large integers or
overly precise floating point to suffer overflow or significance loss,
because it never has to even _think_ about the coordinates of any
point not in the actual output area.
But I resisted the temptation to throw out our existing Penrose
generator and move to the shiny new algorithm, for just one reason:
backwards compatibility. There's no sensible way to translate existing
Loopy game IDs into the new notation, so they would stop working,
unless we kept the old generator around as well to interpret the
previous system. And although _random seeds_ aren't guaranteed to
generate the same result from one build of Puzzles to the next, I do
try to keep existing descriptive game IDs working.
So if we had to keep the old generator around anyway, it didn't seem
worth writing a new one...
... at least, until we discovered that the old generator has a latent
bug. The function that decides whether each tile is within the target
area, and hence whether to make it part of the output grid, is based
on floating-point calculation of the tile's vertices. So a change in
FP rounding behaviour between two platforms could cause them to
interpret the same grid description differently, invalidating a Loopy
game on that grid. This is _unlikely_, as long as everyone uses IEEE
754 double, but the C standard doesn't actually guarantee that.
We found this out when I started investigating a slight distortion in
large instances of Penrose Loopy. For example, the Loopy random seed
"40x40t11#12345", as of just before this commit, generates a game
description beginning with the Penrose grid string "G-4944,5110,108",
in which you can see a star shape of five darts a few tiles down the
left edge, where two of the radii of the star don't properly line up
with edges in the surrounding shell of kites that they should be
collinear with. This turns out to be due to FP error: not because
_double precision_ ran out, but because the definitions of COS54,
SIN54, COS18 and SIN18 in penrose.c were specified to only 7 digits of
precision. And when you expand a patch of tiling that large, you end
up with integer combinations of those numbers with coefficients about
7 digits long, mostly cancelling out to leave a much smaller output
value, and the inaccuracies in those constants start to be noticeable.
But having noticed that, it then became clear that these badly
computed values were also critical to _correctness_ of the grid. So
they can't be fixed without invalidating old game IDs. _And_ then we
realised that this also means existing platforms might disagree on a
game ID's validity.
So if we have to break compatibility anyway, we should go all the way,
and instead of fixing the old system, introduce the shiny new system
that gets all of this right. Therefore, the new penrose.c uses the
more reliable combinatorial-coordinates system which doesn't deal in
integers that large in the first place. Also, there's no longer any
floating point at all in the calculation of tile vertex coordinates:
the combinations of 1 and sqrt(5) are computed exactly by the
n_times_root_k function. So now a large Penrose grid should never have
obvious failures of alignment like that.
The old system is kept for backwards compatibility. It's not fully
reliable, because that bug hasn't been fixed - but any Penrose Loopy
game ID that was working before on _some_ platform should still work
on that one. However, new Penrose Loopy game IDs are based on
combinatorial coordinates, computed in exact arithmetic, and should be
properly stable.
The new code looks suspiciously like the Spectre code (though
considerably simpler - the Penrose coordinate maps are easy enough
that I just hand-typed them rather than having an elaborate auxiliary
data-generation tool). That's because they were similar enough in
concept to make it possible to clone and hack. But there are enough
divergences that it didn't seem like a good idea to try to merge them
again afterwards - in particular, the fact that output Penrose tiles
are generated by merging two triangular metatiles while Spectres are
subdivisions of their metatiles.
I wanted these in order to try to check whether all the faces of a
grid were being traversed in the right orientation. That turned out
not to be the cause of my problem, but it's still useful information
to put in diagnostics.
Previously, the 'faces', 'edges' and 'dots' arrays in a grid structure
were arrays of actual grid_face, grid_edge and grid_dot structures,
physically containing all the data about the grid. But they also
referred to each other by pointers, which meant that they were hard to
realloc larger (you'd have to go through and rewrite all the pointers
whenever the base of an array moved). And _that_ meant that every grid
type had to figure out a reasonably good upper bound on the number of
all those things it was going to need, so that it could allocate those
arrays the right size to begin with, and not have to grow them
painfully later.
For some grid types - particularly the new aperiodic ones - that was
actually a painful part of the job. So I think enough is enough:
counting up how much memory you're going to need before using it is a
mug's game, and we should instead realloc on the fly.
So now g->faces, g->edges and g->dots have an extra level of
indirection. Instead of being arrays of actual structs, they're arrays
of pointers, and each struct in them is individually allocated. Once a
grid_face has been allocated, it never moves again, even if the array
of pointers changes size.
The old code had a common idiom of recovering the array index of a
particular element by subtracting it from the array base, e.g. writing
'f - g->faces' or 'e - g->edges'. To make that lookup remain possible,
I've added an 'index' field to each sub-structure, which is
initialised at the point where we decide what array index it will live
at.
This should involve no functional change, but the code that previously
had to figure out max_faces and max_dots up front for each grid type
is now completely gone, and nobody has to solve those problems in
advance any more.
Now using the browser's "copy" operation while the focus is in the
puzzle will copy the puzzle state to the clipboard. Browsers seem to
have odd ideas about whate element to target with the "copy" event:
Firefox targets the parent of the <canvas> while Chromium targets the
<body>. To cope with these and possible future weirdness I attach the
event handler to the document and then look to see if it's plausibly
related to the canvas.
Arguably we might want to handle a wider range of "copy" events, maybe
any where the selection isn't empty. I'm not sure, though, so we'll
start with the minimal change.
I missed this in my previous addition of preferences for UI controls
(4227ac1fd5dc25c247e7526526079b85e1890766) because it wasn't documented.
Now it is documented and it has a preference.
I've phrased it as showing possible bridge locations, which doesn't
really make clear that "possible" relates only to the locations of
islands and not to anything already placed. Improvements welcome!
The change I made in c224416c76e41f284b318adc51f08c3ed11de8e2 was
incorrect: I accidentally removed a "return" statement and left in a
debugging printf() when I meant to keep the return and drop the
printf().
The boundary between them for mouse clicks probably wants to be
revisited because at the moment it's slightly inside the edge of the
grid. I tried using INUI() instead of INGRID() but that just gives a
different wrong answer, so I may need to actually understand what's
going on here.
In Pearl, a mouse-down outside the grid sets ui->ndragcoords to -1.
The intended effect of this is to make sure that future drags are
ignored, so you can't try to draw a line starting off the grid.
However, this also has the effect of clearing any in-progress drag.
This can happen if there's a keyboard "drag" in progress at the time.
This is arguably wrong, but much more wrong was that interpret_move
returned MOVE_UNUSED (and previously NULL) in this case. That meant
that the display didn't get updated to show the abandonment of the
drag, or the removal of the keyboard cursor that also happened. This
commit changes MOVE_UNUSED to MOVE_UI_UPDATE so that at least the
effect is correctly visible.
Chris Boyle points out that outline_block_structure has a comment
saying that we're supposed to have picked a square with a boundary to
its left. dsf_canonify no longer guarantees that, but dsf_minimal
does. Switch to using that throughout the function.
'keen --generate 10#12345 --print 5x2' failed an assertion before this
fix, and now doesn't.
midend_process_key() has some generic code for converting MOD_CTRL along
with a printing character into a control character. This is derived
from the Emscripten front-end because browsers don't do this themselves.
Most other front ends seem to depend on the platform for this mapping.
The mapping was applied to all printable ASCII characters, but this
meant that Ctrl+-, which is commonly used by browsers to mean "zoom out"
got converted into CR and then CURSOR_SELECT. That was confusing to say
the least.
So now, the CTRL mapping is only applied to characters in the roughly
alphabetic range (0x40 to 0x7f), and MOD_CTRL applied to a character in
the range 0x20 to 0x3f gets a return of PKR_UNUSED instead. That means
that Ctrl+- in browsers now works properly.
I don't think this will affect other front-ends because they're
generally a lot less generous about passing MOD_CTRL to the mid-end.
I've tested the GTK port nonetheless and not found any problems.
The only tricky bit is whether clicking precisely on the diagonal of a
square (which never has any effect on the game) is MOVE_UNUSED or
MOVE_NO_EFFECT. I decided that having single-pixel lines in the middle
of the grid causing events to be passed back to the environment would be
very confusing, so they're MOVE_NO_EFFECT. Clicking entirely outside
the grid, on the other hand, returns MOVE_UNUSED.
Slightly more complicated than usual, because Pegs has irregularly
shaped grids so detecting whether a click is inside requires more than
just a range check.
Also fixed a typo in a nearby comment.
Integer division in C rounds towards zero, so if you want it to
consistently round down you need to ensure that the arguments are
positive. FROMDRAW() didn't do that, so clicks off the top and left
corners of the grid got treated as being in the top row or left column
(row and column 0) rather than ignored.
This commit fixes the macro so that it offsets its argument upward
before the division and compensates afterwards.
This is the most efficient way to apply the combinatorial coordinate
system. As described in my original article (and mentioned again in
the followup one), you can walk along a horizontal or vertical line of
the tiling, identifying which edge of each tile the line will leave it
by, and computing the location and coordinates of the next tile beyond
that edge, so that you visit every tile intersected by the edge.
By doing one iteration, say vertically up the left of your target
area, and using the tiles you find as starting points for a series of
perpendicular sub-iterations spaced closely enough not to miss any
tiles, you can arrange to visit every tile intersecting your target
region, without having ever had to store a large BFS queue of tiles
left to visit. You only have to keep a small bounded number of
coordinate variables for the whole run, so you can generate a very
large patch of tiling with minimal memory and CPU time.
You can even arrange not to emit any duplicates, without having to
actually store the tiles you've already visited, by checking whether
the y-coordinate of the previous horizontal iteration will have
visited the same tile already.
For Spectres, an extra wrinkle (almost literally) is that they're not
convex, so a horizontal line can visit the same one twice, with
another tile in between. So another part of de-duplication is noticing
_that_: is the edge through which we've just entered this tile the
first one we would have seen while traversing our line? If not, then
trust that it's been emitted already.
As a proof of concept (since I claimed it would work in my writeup
article), and in case anyone wants larger tilings than actual Loopy
will conveniently give you, I've implemented that algorithm in
spectre-test.
However, the actual grid generation for Loopy still uses the more
memory-intensive breadth-first search: because that's what I
implemented first (it's more likely to detect its own errors); because
if I changed it now it would invalidate game descriptions (from all of
two days' worth of live play, but even so); and because the linear
space requirement isn't an important cost for Loopy, which is actually
going to _store_ the whole grid after it's generated, so it needed
linear space _anyway_.
Previously, you'd ask this function 'What lies on the other side of
edge #i of this Spectre tile?' and it would tell you the identity of
another Spectre. Now it will also tell you which _edge_ of that
Spectre adjoins the specified edge of the input one. This will be used
in the extra spectre-test mode I'm about to add.
This uses a tile shape very similar to the hat, but the tiling
_structure_ is totally changed so that there aren't any reflected
copies of the tile.
I'm not sure how much difference this makes to gameplay: the two
tilings are very similar for Loopy purposes. But the code was fun to
write, and I think the Spectre shape is noticeably prettier, so I'm
adding this to the collection anyway.
The test programs also generate a pile of SVG images used in the
companion article on my website.
A test-build with a modern clang points out a number of 'set but not
used' variables, which clang seems to have got better at recently.
In cases where there's conditioned-out or commented-out code using the
variable, I've left it in and added a warning-suppressing cast to
void. Otherwise I've just deleted the variables.
The 'core' library contains almost all the same objects as 'common',
but leaves out hat.c. And the auxiliary program 'hatgen' now links
against that slightly reduced core library instead of 'common'.
This avoids a dependency loop: one of hatgen's jobs is to generate
hat-tables.h, but hat-tables.h is a dependency of it.
Of course, the generated hat-tables.h is already committed, so this
doesn't present a bootstrapping problem in a normal build. But if
someone modifies hatgen.c in order to regenerate hat-tables.h, and
does so in a way that makes it uncompilable, they can't rebuild hatgen
and try again! Of course you can always revert changes with git, but
it's annoying to have to. Better to keep the dependencies non-cyclic
in the first place.
This removed the "handled" pointer and instead extends the existing
boolean return value (quit or don't quit) into an enumeration. One of
the values still quits the program, but now there are different values
for keys that had an effect, had no effect, and are not used by the
puzzle at all. The mapping from interpret_move results to process_key
results is roughly:
move string -> PKR_SOME_EFFECT
MOVE_UI_UPDATE -> PKR_SOME_EFFECT
MOVE_NO_EFFECT -> PKR_NO_EFFECT
MOVE_UNUSED -> PKR_UNUSED
The mid-end can also generate results internally, and is the only place
that PKR_QUIT can arise.
For compatibility, PKR_QUIT is zero, so anything expecting a false
return value to mean quit will be unsurprised. The other values are
ordered so that lower values indicate a greater amount of handling of
the key.
These allow for distinguishing the case where a puzzle doesn't have a
use for a key from the case where it just happens to have no effect in
the current state of the puzzle. These were both represented by NULL,
but that now represents the case where a puzzle hasn't been updated to
make the distinction yet.
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().
Some puzzles have keys that make changes to the display style in ways
that would probably have been user preferences if they had existed.
I've added a user preference for each of these. The keys still work,
and unlike the preferences can be changed without saving any state.
The affected settings are:
* Labelling colours with numbers in Guess ("L" key)
* Labelling regions with numbers in Map ("L" key)
* Whether monsters are shown as letters or pictures in Undead ("A" key)
The C stack used by Emscripten is quite small, so passing more than a
few klilobytes of data on it tends to cause an overflow. Current
versions of puzzles will only generate tiny preferences files, but this
might change in future and in any case Puzzles shouldn't crash just
because the preferences in local storage have got corrupted.
To fix this, we now have JavaScript allocate a suitable amount of C heap
memory using malloc() and stick the preferences file in there.
This could plausibly fail if the preferences file were really big, but
that's unlikely since browsers generally limit an origin to about 5 MB
of local storage. In any case, if malloc() does fail, we'll just ignore
the preferences file, which is probably the right thing to do.
Trying to access window.localStorage will generate an exception if the
local storage is for some reason inaccessible. This can be
demonstrated in Firefox by configuring it to block a site from using
site data. Writing to local storage might also cause an exception if,
for instance, the quota of data for a site is exceeded.
If an exception is raised while loading preferences we log the fact
but don't make any report to the user, behaving as if no preferences
were found. This avoids annoying the user on every visit to a puzzle
page if they're not trying to use preferences.
If something goes wrong when saving, we do currently report that to
the user in an alert box. This seems reasonable since it's in
response to an explicit user action.
If the puzzle canvas is at a ludicrously small size, so that you
attempt to use a zero-height font, then obviously nothing sensible
will appear in the way of text, but you'd at least like to avoid a
crash. But currently, js_canvas_find_font_midpoint will make a canvas,
print some height-0 text into it, and try to retrieve the image pixels
to see what the actual font height was - and this will involve asking
getImageData for a zero-sized rectangle of pixels, which is an error.
Of course, there's only one possible return value from this function
if the font height is 0, so we can just return it without going via
getImageData at all.
(This crash can be provoked by trying to resize the puzzle canvas to
Far Too Small, or by interleaving canvas resizes with browser-tab
zooming. I've had one report that it also occurs in less silly
situations, which I haven't been able to reproduce. However, this
seems like a general improvement anyway.)
dot_bbox() wasn't taking into account the new size of the dots, so
sometimes a rectangle of the grid would be redrawn without a partial
dot it should have contained, because nothing had noticed that that
dot overlapped that rectangle.
Actually I'm not sure why this bug wasn't happening _before_ I
enlarged the dots, because the previous code seemed to think dots had
a fixed size in pixels regardless of tile size, which wasn't even
true _before_ my recent commit 4de9836bc8c36cd. Perhaps it did occur,
just never while I was watching.
Preferences that adjust the display, such as Pearl graphics style or
Light Up lit-blobs toggling, shouldn't affect the official icons, even
if a ~/.config/sgt-puzzles exists in the account that builds the
puzzles.
A user pointed out today that IDM_PREFS overlaps the second preset,
because I forgot that IDM_PRESETS was not a single id but the base for
an open-ended series.
Looking more closely, there are several other problems with the IDM_*
constants. IDM_GAMES (used in COMBINED mode) shouldn't be at an
arbitrary distance _above_ IDM_PRESETS, because that risks a
collision; instead, the games' and presets' ids should interleave.
Also, the ids above IDM_GAMES were going up in steps of 1, which
should have been 0x10, for the usual reason that the bottom four bits
of the id aren't guaranteed. And IDM_KEYEMUL was completely unused (I
suspect it was part of the discarded WinCE support).
Now the #defines that are the bases of series are labelled as
IDM_FOO_BASE; they interleave as they should; and there's a clear
comment.
Requested by a user who otherwise found themself spending too much
time struggling to get lines nicely horizontal or vertical.
The implementation is easy, but the question is what size of grid is
appropriate. Untangle's own generated games are constructed by making
a planar graph drawn on an extremely coarse grid - but snapping to
_that_ grid would give away information about the puzzle solution, and
also, Untangle wouldn't know any similar information about graphs
generated by any other method.
So a better approach is to choose a size of grid that guarantees that
_any_ graph with n vertices can be drawn on it with nonintersecting
straight edges. That sounds like a tricky maths problem - but happily,
the solution is given in a book I already had a copy of. References in
a comment (plus a proof of a pedantic followup detail about multiple
planar embeddings).
Where possible, that is. If our compilation environment has provided
int64_t, we can just make our int64 type be that, and not have to mess
around with multi-word arithmetic at all.
Just spotted this in puzzles.h. We don't need to guess any more from
the C standards version whether stdint.h is available: we've actually
checked _precisely that_ in cmake, so it's better to use the answer.
In the Hats tiling, each tile has two consecutive edges collinear.
When both edges are turned on, i.e. drawn in black just like the dot,
it becomes _just slightly_ tricky to spot the dot in the middle of
that straight line, which is important if you're counting edges around
the face to check a clue.
Increasing the radius from 2/32 to 3/32 of tile size is far too big.
2.5/32 seems reasonable, though.
