The read function used by midend_deserialise and friends is expected
never to perform a partial read (the main deserialisation code always
knows how many bytes it can expect to see), so it's specified to
return simply TRUE or FALSE for success/failure, rather than the
number of bytes read.
This probably wasn't breaking anything, since in the case of
deserialising from an internal memory buffer a short read could only
arise due to an outright bug constructing the buffer. But now I've
spotted it, I should fix it.
This function gives the front end a way to find out what keys the back
end requires; and as such it is mostly useful for ports without a
keyboard. It is based on changes originally found in Chris Boyle's
Android port, though some modifications were needed to make it more
flexible.
This is another situation in which the midend's state, at the time
midend_new_game tries to save it, is not such as to generate a viable
serialisation. In this case, it's because after the user enters a game
id into the Game > Specific or Game > Random Seed dialog box,
midend_set_config will have _already_ started overwriting important
fields of the midend such as me->desc and me->seed, before
midend_new_game is even entered.
In fact this caused an assertion failure (thanks to Lennard Sprong for
reporting it), because one of those fields was set to NULL, so the
resulting serialisation buffer was incomplete, leading to a
deserialisation error later. But I was lucky: if I hadn't been, the
serialisation might have been complete, and valid according to the
deserialisation code, but would have contained a mixture of the new
game's description and the old one's move chain, which would surely
have had far stranger results.
For the moment, I'm fixing this by simply not storing a serialisation
in that situation. Perhaps a nicer approach might be to store one
before starting to overwrite midend fields, and then have
midend_new_game swap it in if it's already been generated. That way
you _could_ still undo past an action like this. But preventing the
assertion failure is a good start.
Now, when we undo a New Game by deserialising the stored version of
the previous game, we start by serialising the _current_ game into a
second serialisation buffer in the midend. Then, if the user tries to
redo back past that undo action, we can re-serialise the earlier game
and re-deserialise the newer one.
A few users had complained (with various degrees of politeness) at the
lack of this ability, because in true xkcd #1172 style, it broke their
workflow. Specifically, if you were a fan of holding down 'u' to undo
all the way back to the start of your current game, you'd have
overshot into the previous game and then have no way to return to the
game you wanted to be at the start of.
This slightly changes the previous interaction of Redo with New Game.
Previously, New Game would save the entire undo chain of the
serialised game, including anything forward of the current position;
so if you took actions move1,move2,undo,newgame then the serialised
game state would include both move1 and move2 with the current
position between them; hence, an undo would restore the old game to a
position just after move1, and then a redo action would re-enact
move2. Now, midend_purge_states is called before serialising the old
game, so in that scenario move2 would be completely lost as a side
effect of the new-game action. (Just as it would be if you made any
_other_ undoable move in that situation.)
Conversely, making a move in the old game after you've undone back
into it will now wipe out the record of the later game, so you can't
redo into it any more.
The three related buf/len/size fields are now a sub-structure of the
main midend, and the serialise/deserialise functions that address
those fields now take a pointer to that sub-structure rather than to
the midend as a whole. This will make it easy for me to drop in a
second substructure alongside it, for redo, and reuse those read and
write functions by just changing the context pointer.
The initial call to midend_new_game() was creating a partial
serialisation containing no game states at all, which meant that if
your first UI action was an undo operation, the game would try to
deserialise that and complain that it was incomplete. Now we detect
that in advance and don't create a useless serialisation in the first
place.
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.
Now midend.c directly tests the returned pointer for equality to this
value, instead of checking whether it's the empty string.
A minor effect of this is that games may now return a dynamically
allocated empty string from interpret_move() and treat it as just
another legal move description. But I don't expect anyone to be
perverse enough to actually do that! The main purpose is that it
avoids returning a string literal from a function whose return type is
a pointer to _non-const_ char, i.e. we are now one step closer to
being able to make this code base clean under -Wwrite-strings.
The newgame_undo data was being saved on every call to
midend_new_game, including the one just after midend_set_params when a
new puzzle preset was selected. So you could select a new preset from
the menu, and then press 'u', and the midend would _try_ to undo that
operation and restore the previous game with a different set of
parameters.
This would do the wrong thing in the front end, because front ends in
general will not be expecting that a change of game parameters might
result from an arbitrary keyboard event - they won't be expecting to
have to call the function that moves the highlight in the game-type
menu, for example, and they _certainly_ won't be expecting that a
window resize might be necessary in response to a random keystroke.
One possible response would be to fix all the front ends so that they
_are_ prepared for either of those consequences of a keystroke event,
and then it would be possible to undo not only the New Game menu
option and the 'n' key but also undo any selection of a preset from
the game-type menu, or even a full re-customisation of the game
settings. But that would be quite an upheaval even in _my_ front end
collection, and also probably be awkward for downstream front ends, so
until I'm convinced of the value of going to all the effort, the
simpler approach is just to disallow undoing a new game in those
situations.
(This does mean that re-selecting the _already active_ game preset
from the type menu will be treated as an undoable new-game event,
which I think is an acceptable UI oddity.)
I've renamed the new midend variables to match my usual naming
convention of using 'size' for the total buffer size and 'len' for the
amount of currently used space (and a couple of other variables to
match those in turn), partly for consistency and also because the name
'midend_undo_used' made me half-expect it to be a boolean. The buffer
itself is called 'midend_undo_buf', again to avoid making it sound
like a function or flag.
Buffer growth is still geometric but less aggressive (factor of 5/4
each time instead of 2), in the hope of wasting less memory on low-RAM
platforms; newgame_undo_deserialise_read should have been static, and
now is; newgame_undo_buf is initialised using NULL rather than 0 so it
doesn't look like an integer, and is freed with the rest of the
midend.
And I think we _should_ enforce by assertion that midend_deserialise
didn't return an error, because there's no reason it ever should in
this situation (unlike when reading from a file, where the user might
have provided the wrong file or a corrupted one). This immediately
allowed me to spot a bug in the existing deserialisation code :-)
It is annoying when one intends to choose "restart" and chooses "new
game" instead. Right now, the puzzle one wanted to try again is
discarded.
To fix this we are going to have to save a lot more information than a
normal game state. Handily, we already have the serialise/deserialise
machinery.
The advantage of using this is that the previous game is easily saved
in its entirety, including its own undo history, and also probably in
a more compact format.
The (de)serialisation interface is rather clunky for use with a memory
target. Sadly none of the existing implementations of a resizing
memory array have been conveniently brought out into puzzles.h, and I
think that that's beyond the scope of what I wanted to do here.
We don't serialise the new game undo serialisation data. So
loading/saving doesn't preserve any "new game" undo, as does "new
game" twice (and as does context switching on a Palm Pilot).
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
This will let me do a 'conditional deserialisation' operation, in
which we fully decode the serialised data and then (assuming that gave
no errors) decide whether or not to actually install it based on some
arbitrary condition.
I don't think there's any possible use for the extra check function
_outside_ midend.c, so I've left the API for front ends as it is; the
externally visible midend_deserialise() doesn't have the new
parameter, and only midend_deserialise_internal() includes it.
Lots of the local variables in midend_deserialise are now fields of a
structure which contains everything that is _going_ to be written into
the midend once we finish validating it all. This makes it easy to
keep all that data together, and (in future) pass it to other
functions all in one go.
No functional change.
The serialised game stores a long-term and a short-term parameter
structure, which correspond to me->params (the thing that gets used by
the next New Game command) and me->curparams (the thing that _was_
used to generate _this_ game). So data relevant to the current game
ought to be validated against the latter, but in fact I was
accidentally passing the former to several validation calls.
I think this probably avoided causing a problem because typically
params and cparams don't differ very much: the usual reason why
they're not the same is that somebody has manually entered a game
description involving an incomplete description of the parameters
(lacking generation-specific details like difficulty level), but by
the very fact that those incomplete descriptions have to contain
_enough_ information to understand a specific game description,
copying just those parts of the description into the long-term params
structure makes the two similar enough that validation won't fail.
However, testing an upcoming patch which calls midend_deserialise at a
more difficult moment (specifically, just after midend_set_params,
meaning that the two params structures can now differ _arbitrarily_)
reveals my error. Fixed to use cparams where that's the right thing.
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.)
That's a case in which the current game IDs have changed, so the
midend ought to be calling the front-end function (if any) that
notifies it when that happens.
The only front end of mine that was affected by this missing call was
the Javascript one, which uses that callback to update the 'Link to
this puzzle' links below the game canvas - but, of course, that front
end didn't ever call midend_deserialise until this month, so no wonder
I never noticed before.
(But downstream front ends might be affected too, for all I know.)
Rockbox's sprintf() lacks the ability to left-justify a string. Fixed
by adding a copy_left_justfied() function to misc.c.
This is a new version of this commit, as the previous version broke
saving!
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.
Net provides the best demonstration of why. Complete a game of net,
then press N while the victory flash is playing: then the victory
flash keeps playing on the new game board. (Tip: save a game which
but for a redo is completed, then you can reproduce this repeatedly
without having to complete a new game each time.)
The flash timer reset code is placed together with the animation
timer reset code, because the two are conceptually related. Note
that midend_restart_game resets animations via midend_finish_move.
This is already done in midend_restart_game via midend_finish_move.
If it's good enough for restarting a game, it ought to also be good
enough for starting new games.
Restarting a game that is already in the restarted state is meant to
be a no-op. It stopped animations. Don't do this.
Also, given that midmidend_restart_game called midend_stop_anim
twice, the invocation we remove was redundant.
Animations were stopped if a new game was initiated with a keyboard
shortcut (n, N, Ctrl-N), but not via menu items such as presets or
custom configurations, nor (perhaps not a problem) on starting the
program. Fix this, so that animations are stopped on a new game no
matter how the new game is started.
In conversation with a user last week, it emerged that the command
'solo --generate 1 9jk#12345' was giving a different game from the one
it gave when I ran it, and it turns out that this is because I've set
SOLO_DEFAULT=7jxdi in my environment to make GUI Solo automatically
start up in my (current) favourite mode. And the difficulty setting
from that parameter string was being reused to fill in the unspecified
difficulty slot in the '9jk', so that the same params string was being
interpreted differently by our two machines.
This is certainly wrong - the whole point of random seed strings like
that is to be interpreted the same way everywhere. But it's a side
effect of something I did do on purpose, for people switching back and
forth between playing randomly generated games and playing a game id
pasted (or typed) in from elsewhere. So this fix, with a giant comment
explaining it, I _think_ should retain the behaviour I originally
wanted while getting rid of the behaviour I didn't.
the GTK front end, plus a 'make test' target in the GTK makefile which
uses them to automatically generate 100 puzzles for each game at each
preset configuration, test-run them back through the solver without
their aux_info to ensure that can cope, and produce an HTML box plot
of game generation times for each preset.
As part of this work I've removed the TESTSOLVE mechanism from r9549,
since the new --test-solve option does the same thing better (in that
when something goes wrong it prints the random seed that caused the
problem).
[originally from svn r9825]
[r9549 == 5a095b8a08fa9f087b93c86aea0fa027138b028d]
when the user pressed 'n' for a new game, because all the front end
knows is that it passed a keystroke to the puzzle, and it has no way
of hearing back that a particular keypress resulted in a game id
change.
To fix this, I've renamed midend_request_desc_changes to
midend_request_id_changes and expanded its remit to cover _any_ change
to the game ids. So now that callback in the Emscripten front end is
the only place from which update_permalinks is called (apart from
initialising them at setup time), and that should handle everything.
[originally from svn r9805]
can trigger a call to a front end notification function. Use this to
update the game ID permalink when Mines supersedes its game ID.
[originally from svn r9793]
The Windows puzzles now accept similar command-line syntax to the GTK
ones, in that you can give them either a game ID (descriptive, random
or just plain params) or the name of a save file. Unlike the GTK ones,
however, the save file interpretation is tried first; this is because
some puzzles (e.g. Black Box) will interpret any old string as a valid
(if boring) game ID, and unlike the GTK puzzles it's not feasible to
require users to disambiguate via a command-line option, because on
Windows a thing that might easily happen is that a user passes a save
file to a puzzle binary via 'Open With' in the GUI shell, where they
don't get the chance to add extra options.
In order to make this work sensibly in the all-in-one Windows app, I
had to get round to another thing I've been planning to do for a
while, which is to write a function to examine a saved game file and
find out which puzzle it's for. So the combined Windows binary will
auto-switch to the right game if you pass a save file on its command
line, and also if you use Load while the program is running.
Another utility function I needed is one to split the WinMain single
command line string into argv. For this I've imported a copy of
split_into_argv() from Windows PuTTY (which doesn't affect this
package's list of copyright holders, since that function was all my
own code anyway).
[originally from svn r9749]
solver I've just modified, by forcing every game generation to be
instantly followed by an attempt to re-solve the same game
_description_ without the aux_info.
I've hacked similar changes in to midend.c several times in the last
couple of months for one reason or another, and it's about time I
arranged not to have to recompile to do it!
[originally from svn r9549]
midend_status(), and given it three return codes for win, (permanent)
loss and game-still-in-play. Depending on what the front end wants to
use it for, it may find any or all of these three states worth
distinguishing from each other.
(I suppose a further enhancement might be to add _non_-permanent loss
as a fourth distinct status, to describe situations in which you can't
play further without pressing Undo but doing so is not completely
pointless. That might reasonably include dead-end situations in Same
Game and Pegs, and blown-self-up situations in Mines and Inertia.
However, I haven't done this at present.)
[originally from svn r9179]
state is in a solved position, and a midend function wrapping it.
(Or, at least, a situation in which further play is pointless. The
point is, given that game state, would it be a good idea for a front
end that does that sort of thing to proactively provide the option to
start a fresh game?)
[originally from svn r9140]
Guess, because Guess expected ^H whereas GTK generated ^?. Other
puzzles that use Backspace do it by being prepared to see either,
which seems wasteful. Now the midend normalises both into ^H, so
front ends can generate whichever they like while puzzles can
safely just look for ^H.
[originally from svn r8786]
alongside the individual puzzle binaries, on Windows only. (MacOS
already has it, of course; Unix would require about as much work
again.)
[originally from svn r8396]
a game which comes with an aux string, we immediately self-test that
string by passing it to solve() and test by assertion that it
succeeded. So a bug in a back end which intermittently generates
malformed aux strings will be detected as soon as it occurs, instead
of only if the user happens to use the Solve operation on a
particular game in which it happened.
Secondly, Ctrl-S now (undocumentedly) triggers the Solve operation,
on the general principle that keyboard shortcuts tend to come in
handy, and on the specific principle that if you want to look at
lots of solved grids in quick succession (say, when observing their
general shape and nature to see if your generation algorithm was
good or not) it's handy to have a quick way of getting to them.
[originally from svn r8298]
actual behaviour change: Untangle now permits dragging with the
right mouse button, which has exactly the same effect as it does
with the left. (Harmless on desktop platforms, but helpful when
"right-click" is achieved by press-and-hold; now the drag takes
place even if you hesitate first.)
[originally from svn r8177]
_conditionally_ able to format the current puzzle as text to be sent
to the clipboard. For instance, if a game were to support playing on
a square grid and on other kinds of grid such as hexagonal, then it
might reasonably feel that only the former could be sensibly
rendered in ASCII art; so it can now arrange for the "Copy" menu
item to be greyed out depending on the game_params.
To do this I've introduced a new backend function
(can_format_as_text_now()), and renamed the existing static backend
field "can_format_as_text" to "can_format_as_text_ever". The latter
will cause compile errors for anyone maintaining a third-party front
end; if any such person is reading this, I apologise to them for the
inconvenience, but I did do it deliberately so that they'd know to
update their front end.
As yet, no checked-in game actually uses this feature; all current
games can still either copy always or copy never.
[originally from svn r8161]
