If you want to remove your saved preferences for a puzzle for any
reason (perhaps because of one of those unsympathetic managers, or
perhaps because it's become corrupted in some way), you can of course
manually go and find the config file and delete it. But if you're not
sure where it is, it's helpful to have a method of telling the puzzle
itself to delete the config file.
Perhaps it would be useful to expose this in the GUI as well, though
I'm not quite sure where is the best place to put it.
(_Perhaps_ it would also be useful to have a more thorough option that
deleted _all_ puzzles' configurations - although that would involve
telling every separate puzzle binary in this collection what all the
other ones' names are, which would be an entirely new build headache.)
Finally, some user-visible behaviour changes as a payoff for all that
preparation work! In this commit, the GTK puzzles get a 'Preferences'
option in the menu, which presents a dialog box to configure the
preference settings.
On closing that dialog box, the puzzle preferences are enacted
immediately, and also saved to a configuration file where the next run
of the same puzzle will reload them.
The default file location is ~/.config/sgt-puzzles/<puzzlename>.conf,
although you can override the .config dir via $XDG_CONFIG_HOME or
override the Puzzles-specific subdir with $SGT_PUZZLES_DIR.
This is the first commit that actually exposes all the new preferences
work to the user, and therefore, I've also added documentation of all
the current preference options.
This fixes a build failure introduced by commit 2e48ce132e011e8
yesterday.
When I saw that commit I expected the most likely problem would be in
the NestedVM build, which is currently the thing with the most most
out-of-date C implementation. And indeed the NestedVM toolchain
doesn't have <tgmath.h> - but much more surprisingly, our _Windows_
builds failed too, with a compile error inside <tgmath.h> itself!
I haven't looked closely into the problem yet. Our Windows builds are
done with clang, which comes with its own <tgmath.h> superseding the
standard Windows one. So you'd _hope_ that clang could make sense of
its own header! But perhaps the problem is that this is an unusual
compile mode and hasn't been tested.
My fix is to simply add a cmake check for <tgmath.h> - which doesn't
just check the file's existence, it actually tries compiling a file
that #includes it, so it will detect 'file exists but is mysteriously
broken' just as easily as 'not there at all'. So this makes the builds
start working again, precisely on Ben's theory of opportunistically
using <tgmath.h> where possible and falling back to <math.h>
otherwise.
It looks ugly, though! I'm half tempted to make a new header file
whose job is to include a standard set of system headers, just so that
that nasty #ifdef doesn't have to sit at the top of almost all the
source files. But for the moment this at least gets the build working
again.
C89 provided only double-precision mathematical functions (sin() etc),
and so despite using single-precision elsewhere, those are what Puzzles
has traditionally used. C99 introduced single-precision equivalents
(sinf() etc), and I hope it's been long enough that we can safely use
them. Maybe they'll even be faster.
Rather than directly use the single-precision functions, though, we use
the magic macros from <tgmath.h> that automatically choose the precision
of mathematical functions based on their arguments. This has the
advantage that we only need to change which header we include, and thus
that we can switch back again if some platform has trouble with the new
header.
After Ben fixed all the unwanted global functions by using gcc's
-Wmissing-declarations to spot any that were not predeclared, I
remembered that clang has -Wmissing-variable-declarations, which does
the same job for global objects. Enabled it in -DSTRICT=ON, and made
the code clean under it.
Mostly this was just a matter of sticking 'static' on the front of
things. One variable was outright removed ('verbose' in signpost.c)
because after I made it static clang was then able to spot that it was
also unused.
The more interesting cases were the ones where declarations had to be
_added_ to header files. In particular, in COMBINED builds, puzzles.h
now arranges to have predeclared each 'game' structure defined by a
puzzle backend. Also there's a new tiny header file gtk.h, containing
the declarations of xpm_icons and n_xpm_icons which are exported by
each puzzle's autogenerated icon source file and by no-icon.c. Happily
even the real XPM icon files were generated by our own Perl script
rather than being raw xpm output from ImageMagick, so there was no
difficulty adding the corresponding #include in there.
I noticed commit db3b531e2cab765a00475054d2e9046c9d0437d3 in the history
where Simon added a bunch of "static" qualifiers. That suggested that
consistently marking internal functions "static" is desirable, so I
tried a build using GCC's -Wmissing-declarations, which requires prior
declaration (presumed to be in a header file) of all global functions.
This commit makes the GTK build clean under GCC's
-Wmissing-declarations. I've also adding "static" to a few obviously
internal objects, but GCC doesn't complain about those so I certainly
haven't got them all.
This adds a new bool * argument, which can be NULL if front ends don't
care whether the keypress was handled. Currently they all do that.
Currently, "undo" and "redo" keys are treated as not handled if there's
no move to undo or redo. This may be a little too strict.
The device pixel ratio indicates how many physical pixels there are in
the platonic ideal of a pixel, at least approximately. In Web browsers,
the device pixel ratio is used to represent "retina" displays with
particularly high pixel densities, and also to reflect user-driven
zooming of the page to different text sizes.
The mid-end uses the device pixel ratio to adjust the tile size at
startup, and can also respond to changes in device pixel ratio by
adjusting the time size later. This is accomplished through a new
argument to midend_size() which can simply be passed as 1.0 in any front
end that doesn't care about this.
- 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.)
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
Apparently some compilers can't work out that new_window() will always
write to its error-message parameter if it returns failure, so they
complain at the call site that 'error' might be used uninitialised.
Fix by explicitly initialising it. (To NULL, which really _shouldn't_
stop the compiler from warning, because surely that's just as bad if
it reaches the following printf!)
Also, while I'm at it, move it into the block where it's used, so it
doesn't look as if it might pervade the whole of main().
I just found out, in GTK3 under X11, that if you use the 'Copy' menu
item to copy a text representation of the game to the clipboard,
afterwards the puzzle window is no longer redrawn.
That was pretty mysterious, and I still don't _fully_ understand it.
But I think the main point is that when you set the GtkDrawingArea to
be the owner of an X11 selection, that requires it to have an X11
window of its own, where previously it was managing fine as a logical
subrectangle of its containing GtkWindow. And apparently switching
strategies half way through the run is confusing to GTK, and causes
redraws to silently stop working.
The easy workaround is to make fe->window rather than fe->area the
thing that owns GTK selections and receives the followup events. That
must already have an X window, so nothing has to be changed when we
make it a selection owner half way through the run.
A user pointed out that if you run a GTK 3 puzzles with "GDK_SCALE=2"
in the environment, the main game drawing area is blurred. That's
because we're choosing the size of our backing Cairo surface based on
the number of _logical_ pixels in the window size, not taking into
account the fact that the non-unit scale factor means the number of
physical pixels is larger. Everything 'works' in the basis - Cairo
happily expands the smaller backing surface into the larger window -
but resolution is lost in the process.
Now we detect the window's scale factor, construct the backing surface
appropriately, and compensate for that scaling when drawing to the
surface and when blitting the surface to the window.
Printing is only available in GTK versions >= 2.10. We can only embed
the page setup dialog on GTK >= 2.18, so on a GTK version less than
that, we must use a separate page setup dialog.
In GTK, printing is usually done one page at a time, so also modify
printing.c to allow printing of a single page at a time.
Create a separate drawing API for drawing to the screen and for
printing. Create a vtable for functions which need to be different
depending on whether they were called from the printing or drawing
API.
When a function is called from the printing API, it is passed a
separate instance of the frontend than if it were called from the
drawing API. In that instance of the frontend, an appropriate vtable
is available depending on whether it was called from the printing or
drawing API.
The low-level functions used for printing are enabled even if printing
is not enabled. This is in case we ever need to use them for something
other than printing with GTK. For example, using Cairo as a printing
backend when printing from the command line. Enabling the low-level
functions even when GTK printing is not available also allows them to
be compiled under as many build settings as possible, and thus lowers
the chance of undetected breakage.
Move the definition of ROOT2 from ps.c to puzzles.h so other files can
use it (gtk.c needs it for hatching).
Also add myself to the copyright list.
[Committer's note: by 'printing', this log message refers to the GTK
GUI printing system, which handles selecting a printer, printing to a
file, previewing and so on. The existing facility to generate
printable puzzles in Postscript form by running the GTK binaries in
command-line mode with the --print option is unaffected. -SGT]
When no icons are available, n_xpm_icons will be 0, and
menu_about_event() will try to access xpm_icons[n_xpm_icons-1]. Since
n_xpm_icons is 0, this becomes xpm_icons[-1] which is an invalid
value, causing a segfault.
Instead, check if n_xpm_icons is 0, and if so, don't pass any icon to
gtk_show_about_dialog().
I had occasion recently to want to take a puzzle screenshot on a
machine that didn't have the GTK3 libraries installed, but is advanced
enough to build the GTK2+Cairo version of the puzzles. That _ought_ to
be good enough to take screenshots using bare Cairo without GTK; I
think the only reason why I didn't bother to support it before is
because on GTK2, frontend_default_colours() queries the widget style
to choose a shade of grey. But we have a fixed fallback shade of grey
to use on GTK3, so it's easy to just use that same fallback in
headless GTK2.
Moving the snaffle_colours() call earlier is fine in GTK 3, where the
potential call to frontend_default_colour doesn't depend on the window
already having been created. But it falls over in GTK 2 where it does.
Moved the non-headless-mode version of that call back to where it was
before the --screenshot change.
I had this idea today and immediately wondered why I'd never had it
before!
To generate the puzzle screenshots used on the website and as program
icons, we run the GTK front end with the --screenshot option, which
sets up GTK, insists on connecting to an X server (or other display),
draws the state of a puzzle on a Cairo surface, writes that surface
out to a .png file, and exits.
But there's no reason we actually need the GTK setup during that
process, especially because the surface we do the drawing on is our
_own_ surface, not even one provided to us by GTK. We could just set
up a Cairo surface by itself, draw on it, and save it to a file.
Calling gtk_init is not only pointless, but actively inconvenient,
because it means the build script depends on having an X server
available for the sole purpose of making gtk_init not complain.
So now I've simplified things, by adding a 'headless' flag in
new_window and the frontend structure, which suppresses all uses of
actual GTK, leaving only the Cairo surface setup and enough supporting
stuff (like colours) to generate the puzzle image. One awkward build
dependency removed.
This means that --screenshot no longer works in GTK 2, which I don't
care about, because it only needs to run on _one_ platform.
This is the main bulk of this boolification work, but although it's
making the largest actual change, it should also be the least
disruptive to anyone interacting with this code base downstream of me,
because it doesn't modify any interface between modules: all the
inter-module APIs were updated one by one in the previous commits.
This just cleans up the code within each individual source file to use
bool in place of int where I think that makes things clearer.
This commit removes the old #defines of TRUE and FALSE from puzzles.h,
and does a mechanical search-and-replace throughout the code to
replace them with the C99 standard lowercase spellings.
This changes parameters of midend_size and midend_print_puzzle, the
return types of midend_process_key, midend_wants_statusbar,
midend_can_format_as_text_now and midend_can_{undo,redo}, the 'bval'
field in struct config_item, and finally the return type of the
function pointer passed to midend_deserialise and identify_game.
The last of those changes requires a corresponding fix in clients of
midend_deserialise and identify_game, so in this commit I've also
updated all the in-tree front ends to match. I expect downstream front
ends will need to do the same when they merge this change.
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.
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.
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.
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.
This is in addition to the existing keystrokes r, ^R and ^Y. I've
become used to Ctrl-Shift-Z in other GUI games, and my fingers keep
getting confused when my own puzzles don't handle it the same way.
This fixes an amusing UI bug that I think can currently only come up
in the unpublished puzzle 'Group', but there's no reason why other
puzzles _couldn't_ do the thing that triggers the bug, if they wanted
to.
Group has unusual keyboard handling, in that sometimes (when a cell is
selected for input and the key in question is valid for the current
puzzle size) the game's interpret_move function will eat keystrokes
like 'n' and 'u' that would otherwise trigger special UI events like
New Game or Undo.
The bug is that fake keypress events generated from the GUI menus
looked enough like those keystrokes that interpret_move would eat
those too. So if you start, say, a 16x16 Group puzzle, select an empty
cell, and then choose 'new game' from the menu, Group will enter 'n'
into the cell instead of starting a new game!
I've fixed this by inventing a new set of special keystroke values
called things like UI_NEWGAME and UI_UNDO, and having the GUI menus in
all my front ends generate those in place of 'n' and 'u'. So now the
midend can tell the difference between 'n' on the keyboard and New
Game from the menu, and so Group can treat them differently too. In
fact, out of sheer overcaution, midend.c will spot keystrokes in this
range and not even _pass_ them to the game back end, so Group
shouldn't be able to override these special events even by mistake.
One fiddly consequence is that in gtk.c I've had to rethink the menu
accelerator system. I was adding visible menu accelerators to a few
menu items, so that (for example) 'U' and 'R' showed up to the right
of Undo and Redo in the menu. Of course this had the side effect of
making them real functioning accelerators from GTK's point of view,
which activate the menu item in the same way as usual, causing it to
send whatever keystroke the menu item generates. In other words,
whenever I entered 'n' into a cell in a large Group game, this was the
route followed by even a normal 'n' originated from a real keystroke -
it activated the New Game menu item by mistake, which would then send
'n' by mistake instead of starting a new game!
Those mistakes cancelled each other out, but now I've fixed the
latter, I've had to fix the former too or else the GTK front end would
now undo all of this good work, by _always_ translating 'n' on the
keyboard to UI_NEWGAME, even if the puzzle would have wanted to treat
a real press of 'n' differently. So I've fixed _that_ in turn by
putting those menu accelerators in a GtkAccelGroup that is never
actually enabled on the main window, so the accelerator keys will be
displayed in the menu but not processed by GTK's keyboard handling.
(Also, while I was redoing this code, I've removed the logic in
add_menu_item_with_key that reverse-engineered an ASCII value into
Control and Shift modifiers plus a base key, because the only
arguments to that function were fixed at compile time anyway so it's
easier to just write the results of that conversion directly into the
call sites; and I've added the GTK_ACCEL_LOCKED flag, in recognition
of the fact that _because_ these accelerators are processed by a weird
mechanism, they cannot be dynamically reconfigured by users and
actually work afterwards.)
To do this, I've completely replaced the API between mid-end and front
end, so any downstream front end maintainers will have to do some
rewriting of their own (sorry). I've done the necessary work in all
five of the front ends I keep in-tree here - Windows, GTK, OS X,
Javascript/Emscripten, and Java/NestedVM - and I've done it in various
different styles (as each front end found most convenient), so that
should provide a variety of sample code to show downstreams how, if
they should need it.
I've left in the old puzzle back-end API function to return a flat
list of presets, so for the moment, all the puzzle backends are
unchanged apart from an extra null pointer appearing in their
top-level game structure. In a future commit I'll actually use the new
feature in a puzzle; perhaps in the further future it might make sense
to migrate all the puzzles to the new API and stop providing back ends
with two alternative ways of doing things, but this seemed like enough
upheaval for one day.
gdk_window_set_background_rgba is deprecated as of GTK 3.22, because
apparently you can't just _say_ any more 'here is what I want my
window's background colour to be in places where a widget isn't'.
Instead you have to provide a GtkStyleProvider which can be slotted
into a wobbly tower of other providers with associated priorities, so
that the user can override your choices if they really want to.
And the easiest way to constructc a GtkStyleProvider in turn is to
write *actual CSS* and get GTK to parse it, so I end up converting my
nice numeric RGB values into a complicated text format for another
part of _the same process_ to parse back into numbers. Sigh.
I've just noticed that the GTK game window was not being redrawn when
changing between puzzle modes that don't involve a window resize - by
selecting a preset from the Type menu (e.g. changing between different
12x12 settings in Flood) or via the Custom menu.
It looks as if the bug was introduced in commit 8dfe5cec3, which
suggests that it was a side effect of the switch from
gtk_window_resize_to_geometry to plain gtk_window_resize. My guess is
that the implementation of the former function inside GTK might have
happened to trigger an unconditional window resize, while the latter
took the shortcut of doing nothing if the window was already the right
size; hence, resize_fe() would have been reliably generating a redraw
event without me having to ask for one, but now it doesn't, so I have
to trigger one myself any time I've just called resize_fe.
Now we work out for ourselves how the drawing-area size relates to the
overall window size, by adding on the height of fe->menubar and/or
fe->statusbar.
gtk_misc_set_alignment was deprecated in GTK 3.14. But my replacement
code using gtk_label_set_{x,y}align doesn't work there, because that
function wasn't introduced until GTK 3.16, so there are two minor
versions in the middle where a third strategy is needed.
(That middle strategy doesn't permit arbitrary float alignments, but
that's OK, bceause we only actually use multiples of 0.5.)
Jonas Kölker points out that commit a800ff16b (which fixed a bug in
the previous attempt) left in another bug: if the puzzle size was
changed while the window was maximised, the system would fail to
recompute the tile size and would redraw completely wrongly.
So it's not optional after all to run midend_size(), even if the
drawing area size hasn't changed. I've reverted that code to be
unconditional, and now only the refresh of the Cairo-side backing
store system is conditionalised - and that's conditionalised on
changes to either the size of the actual window _or_ the size of the
contained pixmap. (The latter should defend against redraw failure in
the case where the puzzle aspect ratio changes, so that neither the
window size nor the tile size changes but a recentre is still needed.)
I _think_ this now fixes all the cases of resizing: this time I've
tested making an unmaximised puzzle window bigger or smaller, and
within a maximised window, forcing the puzzle to scale up, scale down,
or change its aspect ratio without changing its tile size. All work,
on GTK2 and GTK3, and yet we still don't get the visible flicker on
status line updates that was the reason I started fiddling with this
code in the first place.
(We _do_ still call configure_area on every update of the status line,
at least under GTK3; that's going to cause a forced full redraw on
every timer tick in Mines, which is wasteful of CPU, so it would still
be nice to find a better way of identifying the cases in which no
resizing at all was necessary and we could avoid renewing the game
drawstate. But the current code at least doesn't have any display
_errors_ that I know of, which is an improvement on its previous
state.)
- The file selector for loading and saving gets a g_free().
- The handling of saving (menu_save_event) gets an sfree().
- It's also slightly restructured to prevent future errors.
- menu_load_event was already structured to prevent this error.
- The OLD_FILESEL code seems to not need a g_free().
These button codes are generated by the back/forward button pair on
the sides of some mice, and web browsers treat these as the back and
forward actions in the page history.
It's becoming annoying to keep working within the increasing
restrictions on GtkDialog, in particular the fact that not only do we
have to let it have complete control of the button area, but also it's
not clear whether we can intercept a press of the 'OK' button and
display an error message rather than ending the dialog.
So, as I did in PuTTY, I'm resorting to using an ordinary GtkWindow
with controls I laid out myself.
