This requires looking at the CSS size of the puzzle canvas rather than
its internal size, and then adjusting the new size to account for the
device pixel ratio.
Because it's the simplest thing to do, when we notice such a change we
keep the current puzzle at its existing size measured in device
pixels. This has the rather odd consequence that when changing the
text size in Firefox, the size of the puzzle remains constant.
The CSS "px" unit isn't always a device pixel. On devices with
high-DPI displays, there can often be multiple device pixels to a CSS
px, while in particularly low-resolution displays (like feature
phones), the user might zoom out to get several CSS px to a device
pixel. And even on desktop browsers, text zooming controls can change
the ratio.
To make Puzzles' rendering look good on an arbitrary device pixel
ratio, we really want the pixels of the canvas to be device pixels,
not CSS px, so that the canvas doesn't have to be scaled by the
browser for display. To correct this, we now control the CSS size of
the puzzle canvas, via its containing <div>, to be the canvas size
divided by the device pixel ratio.
There is a significant gap, which is that this doesn't yet track
changes to the device pixel ratio. This is slightly complicated, so
I'll put it off to the next commit.
Our system for mapping mouse coordinates to canvas coordinates assumed
that the puzzle canvas had the same dimensions in CSS as its own
internal width and height. This is true in the current wrapper HTML,
but it's very easy to accidentally change and there are circumstances
where we might want to deliberately change it in future.
To fix this, we now inspect the CSS size of the canvas when processing
mouse events, and map the coordinates through the scaling and
translation necessary to convert CSS pixels into canvas pixels.
Most of the old URLs don't work any more. As far as I can see, the
new pages have no Flash, and even if they did very few browsers will
still support it.
This is necessary to allow all random seeds to round-trip properly.
It's probably not currently necessary for descriptive game IDs, but it
won't hurt.
I've deliberately gone for encoding only those characters that are not
valid in fragment identifiers to minimise the ugliness of the generated
URLs. For slightly interesting historical reasons, '#' is not valid in
a fragment identifier, so all random seed links end up a little bit
ugly.
Now that save files are (even more) officially ASCII, it's perfectly
safe to pass them to JavaScript as UTF-8 strings.
This means that the form in which save files are shipped from C to
JavaScript is the same is the form in which they're shipped from
JavaScript to C. That allows for doing new things with them, like
writing them to local storage.
This reverts commit f729f51e475ff98d0caf529f0723ef810b1c88ef.
Net can have non-alphanumeric characters in its parameter strings. Both
"5x5b0.1" and "5x5b1e-05" are valid parameter strings generated by Net.
So only "most" puzzles use alphanumeric parameter strings.
The developer documentation claims that save files are long ASCII
strings. This is mostly true, but there's nothing stopping a user
from entering non-ASCII characters as random seeds. The ASCII
property of save files is useful, so encode seeds in hex before
writing them unless they consist only of printable ASCII characters.
Hex-encoded seeds are written under a new key, HEXSEED, to distinguish
them from unencoded seeds. This means that old versions of the code
won't be able to load encoded seeds, but that's not a great loss:
seeds aren't generally portable between versions anyway.
That they are ASCII is implied by their inclusion in save files.
Nothing requires an absence of control characters, but it seems polite
to make them slightly readable.
Whenever the midend calls encode_params, it also checks that the
result is a printable ASCII string that doesn't contain '#' or ':'.
Parameter strings are embedded in save files, so they have to fit within
ASCII. They can't contain '#' or ':' because those delimit the
parameter section of a game ID. Nothing explicitly says they can't
contain control characters, but those would be a particularly egregious
violation of the recommendation that parameter strings be easy to type
into a shell.
This removes any assumption in the JavaScript code about precisely what
"display" setting the element should have.
This means that now the only places where the JavaScript manipulates
elements' styles are to set the width of the puzzle container and to
mark and unmark elements with "display: none". These both seem like
reasonable kinds of semantic markup that just happen to be expressed as
styles.
An element with display: block will naturally adjust to fit the width of
its container, so if that's what you want there's no need to set its
width explicitly.
Some elements (generally those created by JavaScript) had their style
parameters set directly by JavaScript. Putting styles in CSS generally
makes them easier to understand (and fiddle with), so I've done that.
The only styles left in JavaScript are those that are calculated by
JavaScript (like the status-bar size) and the random-seed permalink
visibility because I wasn't quite sure how to handle it.
statepos == 0 shouldn't ever occur in a save file because it indicates
an uninitialised midend. OTOH statepos == nstates is normal. Also
added an equivalent assertion when saving because Simon and I spent
some time discussing whether it could happen.
Without this, the "Undo" button ends up greyed even though it actually
works. I'm not sure about whether updating the permalinks is necessary:
maybe we don't need that in new_game() either.
Load/save has been in the JavaScript backend for a while, as have
prettier controls. And JavaScript-capable touchscreens are all around
us, if still poorly supported by Puzzles.
In their normal state, most of the top-level menu items are a verb and
an object, like "Undo move". This is admirably clear, but on a small
screen the menu can take a lot of space. In each case the verb alone
is sufficient to know what the button does, so use a media query to
suppress the noun is the viewport is very narrow. "Very narrow" here
is roughly where the menus would overflow onto four lines in my
browser.
In the old design, when they wrapped onto multiple lines, various bad
things happened. The lines overlapped one another, the lines got
broken within buttons but not between buttons, and if they had got
broken between buttons the left button on each line would have lacked
a left border.
I've made two major changes to fix this. First, I've switched from
flow layout to flex layout. This has much better default behaviour,
breaking lines in the right places, not overlapping lines, and even
arranging line-wrapping within a button when the viewport gets really
narrow.
Second, I've given each button a border on all four sides and then
used negative margins to overlap them. This required changing the
borders from transparent black to opaque grey to make them display
correctly when overlapping.
The result is not quite identical to the old version on a wide
viewport, but I think it's as close as I can get while keeping the new
CSS pleasant.
Ideally, the separator would vanish when it was adjacent to a line
break, but I've not worked out how to do that yet.
- Generate HTML pages from the manual, and install them
- Add "Contents" and "Help on <name>" menu items that will open the
appropriate page in a web browser
Ben Hutchings points out that when I 'harmlessly' changed 'dark_theme'
from a gboolean to a bool, it wasn't harmless, because its address is
passed to g_object_get, which expects a pointer to gboolean. (And of
course it's a variadic function, so it can't type-check that.)
It's got a bit out of date over the years, with some changes to the
code not fully reflected in it (e.g. not all the int -> bool type
changes were documented, and TRUE and FALSE were still mentioned),
and quite a lot of new functions not added. (In particular, the dsf
API was not documented, and it certainly should have been, if only so
that people can find out what it even stands for!)
As well as correcting for factual accuracy, two content changes in the
advice chapter:
I've reworded the definition of 'fairness' to explicitly mention that
requiring the player to use Undo is cheating. That's always how I
_intended_ the definition, but I didn't say it clearly enough.
And I've added an entire new section describing the normal sensible
way to implement redraw(), via a loop of the form 'work out what this
cell should look like, check it against an array in game_drawstate of
the last state we drew it in, and if they're different, call a redraw
function'. That was mentioned in passing in two other sections, but I
know at least one developer didn't find it, so now it's less well
hidden.
Reordered the statements in the fixed Unruly blank_state so that there
doesn't need to be a double if statement (and I think it's more
sensible in any case to put each memset of a freshly allocated array
immediately after the alloc).
In GTK set_window_background, the nest of #ifdefs is now complicated
enough to deserve a few comments on the #else and #endif lines. And
while I was there I switched the gboolean to a bool, on my general
principle that platform-specific boolean types are only worth using
when you're passing them to a platform API function (and perhaps not
even then, if it's not passed by reference).
Commit cc7f550 "Migrate to a CMake-based build system." reversed the
order of xpm_icons, so the largest icon (96x96) is now first and
the smallest (16x16) is now last.
The About dialog now shows the smallest icon, and the window icon is
now the largest icon.
Change the array indexing so that the same size icons are used as
before.
Fixes: cc7f5503dc8f ("Migrate to a CMake-based build system.")
When the user chooses a global dark theme, the window background will
be dark and the default text colour light. Overriding the window
background to be light grey can make the menu and status bars
unreadable. In case a dark theme is used, only set the background
colour for the content area.
References: https://bugs.debian.org/944237
WITHIN() used to treat the min and max as inclusive bounds but was
changed to treat the max as exclusive, apparently accidentally.
Fixed: 5f5b284c0bdd ("Use C99 bool within source modules.")
The common structure is ref-counted and dup_game() bumps the reference
count rather than copying it. However, blank_state() always allocates
a new instance. Add a parameter to control whether blank_state()
allocates it.
Fixes: 47cec547e59a ("Unruly, Group: reference-count the 'immutable' array.")
gcc 6 warns about statements that are indented as if they were meant
to be part of a preceding conditional block. In this case I don't
think that was intended, so shift it left.
References: https://bugs.debian.org/811577
Josh Triplett reported:
> If I ask pearl to generate a 5x5 tricky puzzle, it runs forever.
I find that 5x6 or 6x5 works, so set the minimum accordingly.
References: https://bugs.debian.org/667963
interpret_ui_drag is now called from update_ui_drag, so it makes more
sense to have the former appear first in the file, together with its
comment explaining the expected usage.
A user reported recently that they were trying this as an extra
challenge (solve the whole puzzle mentally and then draw it in
finished form in one UI action). But the backtracking behaviour of
Pearl's dragging mode meant that the loop erased itself as soon as the
drag came back to a revisited position.
In this commit I fix that by making the exception that you can
unconditionally return to the start point of the drag, _provided_ that
in doing so you don't create a grid cell of degree > 2.
Every call to draw_cell() was drawing a region including the whole
border of the cell, so that the calls overlapped. So if the cursor
moved left or up, then a COL_CURSOR outline would be drawn around the
new cell, and then a COL_GRID outline would be drawn around the old
cell, overwriting part of the cursor border.
I've fixed this in the rigorous way, by making draw_cell() calls cover
disjoint areas of the puzzle canvas, and using clip() to enforce that.
So now the single DRAWFLAG_CURSOR is replaced by a system of four
flags, indicating that the cell being drawn is the actual cursor
position, or the cell below it (hence containing the cursor's bottom
border), or to its right (needing the left border), or below _and_ to
the right (you still need the single pixel at the cursor's bottom
right corner!).
Also, to ensure the cursor edges are drawn even on the bottom or right
grid boundaries, draw_cell() is called for a set of virtual cells
beyond the actual grid bounds, with additional flags telling it not to
draw an actual puzzle cell there, just the relevant pieces of border.
The purpose of check_window_size is to be run after we try to resize
the puzzle window, decide whether the window size needs a further
update, and if so, make it. But the SetWindowPos call that actually
made the update (triggered by the subroutine check_window_resize
returning true to indicate that a change was needed) had mysteriously
gone missing.
An example case where this goes wrong: start up a puzzle at a game
size large enough to need a tile size smaller than default. Then
change the setting to one so small that the menu bar is now the
limiting factor on how small the window can be. (For example, changing
Mosaic from its 50x50 preset to 3x3, if your monitor isn't so huge
that the former fits.) The window comes out the wrong size, and with
this SetWindowPos reinstated, now it gets corrected.
What seems to have happened was that the SetWindowPos was originally
under #ifndef _WIN32_WCE, i.e. we only want to do it on desktop
Windows, not on CE. Then commit 39d299f579da3e9 (introducing manual
window resizing in the Windows front end) moved the call into a
different function, and in the process, accidentally reversed the
sense of the #ifdef. And then commit ff3e762fd007883 (removing the
bit-rotted Windows CE support completely) removed it, along with
everything else under #ifndef _WIN32_WCE!
It was originally supposed to be _enabled_ on desktop Windows, not
disabled. So I've put it back now.
The default setting for 'aggressiveness' is true. But the extra suffix
to specify it in the full version of the encoded game params was being
set if aggressiveness _was_ true, not if it was false. Result: round
trip encode/decode of a non-aggressive configuration fails, because
the encoded string has no aggressiveness suffix, and the decoder
interprets that as going back into aggressive mode.
Pulled out the default setting into a #define which is checked
consistently in both locations.
I've had a lot of Puzzles nightly builds fail recently in the NestedVM
stage, with a 'jar' command producing a message along these lines:
java.util.zip.ZipException: attempt to write past end of STORED entry
at java.base/java.util.zip.ZipOutputStream.write(ZipOutputStream.java:337)
at jdk.jartool/sun.tools.jar.Main.copy(Main.java:1250)
at jdk.jartool/sun.tools.jar.Main.copy(Main.java:1263)
at jdk.jartool/sun.tools.jar.Main.addFile(Main.java:1211)
at jdk.jartool/sun.tools.jar.Main.create(Main.java:879)
at jdk.jartool/sun.tools.jar.Main.run(Main.java:319)
at jdk.jartool/sun.tools.jar.Main.main(Main.java:1681)
Suppressed: java.util.zip.ZipException: invalid entry size (expected 0 but got 786 bytes)
at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:288)
at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:361)
at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:238)
at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:378)
at jdk.jartool/sun.tools.jar.Main.create(Main.java:854)
... 2 more
It's hard to work out exactly what this error dump means, and
web-searching for the error message isn't much help because the same
exception can occur in application code using java.util.zip, and most
mentions on the web are about that, and not about what I want to know,
which is why it might happen in the 'jar' program in particular.
However, the clues visible in that message suggest that 'jar' had
somehow got confused about the size of one of the files it was adding
to the jar archive, in that it initially decided it was 0 bytes long
and later found it was longer. That suggests a problem of excessive
parallelism between the build steps, perhaps due to a missing
dependency in the makefile, which might plausibly cause the 'jar' step
to be running already while some file it needs to read is still being
written. (Which would also explain why it doesn't happen every time.)
An eyeball review of cmake/platforms/nestedvm.cmake didn't find any
obvious missing dependencies. But I vaguely remembered that in some
other context I'd had trouble with cmake 'add_custom_command'. So in
this commit I replace all those custom commands with custom _targets_,
listing the previous OUTPUT files as BYPRODUCTS. And then the
dependencies are written using the target names, instead of the file
names.
I don't fully understand why this should make a difference. But it
seems more reliable in a soak test, and still builds the right things,
so I'll commit it and see if it makes the flakiness in the actual
nightly builds stop happening.
