Patches for two Aisleriot bugs

[eebfrasn-gngm usa net]

Greetings, oh mighty masters of the Aisleriot game :)

(I had to subscribe to the games-list to post this - but please CC me
directly
on replies as well, as I may un-subscribe soon)

After upgrading from Ubuntu 12.4 to 14.4 (which has aisleriot 3.10.2), I
found
two annoying regressions in my favorite waste of time, a.k.a. Solitaire:

- 'undo' no longer updates the status message (so on Spider, for example,
when
undoing or re-doing past a deal from stock, the 'stock remaining' count isn't
updated)

- dragging cards to the window edge makes a complete mess out of them


Both still exist in the newest sources (3.15.0, as far as I could tell), so I
set out to try and debug them.

My solutions are attached. They are both very simple - though I have to admit
that they might not be up to standard. That's as good as I can do, given that
both Scheme and GTK/GDK are technologies that I have never worked with
before.

More technical detail and possible alternate solution for the missing
give-status-message on undo/redo:

(A)
Scheme/guile and the 'undo' regresion: the change to modules caused the name
spaces of the API (sol.scm, which became api.scm) and the games to become
separate, therefore the call from api.scm to give-status-message always
reaches the definition found in api.scm itself, even if there is an alternate
defined in the game's .scm file. I made api.scm "peek" into the game's main
module and try to call its give-status-message if there is one there.
Admittedly the Guile construct that I used for that is rather messy, but I
did
not find a better way.
I also found another way to achieve the same effect, without using clunky
module lookups - but it requires modifying every game that has
'give-status-message':
- replace (define (give-status-message) ...) in the games with (set!
give-status-message (lambda () ...). This makes it update the definition in
api.scm. To make it actually work, the (define-public (give-status-message)
...) construct in api.scm also has to be modified to use explicit lambda:
(define-public give-status-message (lambda () ...). Even though logically the
shortcut define and the lambda construct should be the same, it appears that
they are not - possibly, when the (define (function) ) syntax is used, Guile
"inlines" the call to give-status-message within api.scm, making the override
from the games ineffective.

(B)
the "smearing cards" problem: After much poking around to try to spot the
bug,
I added gdk_window_set_debug_updates(1) on drag_begin() - and to my horror,
GDK clearly showed that the cards window is actually NEVER painted. The
current code appears to work (somewhat) simply because the update of the card
stack to remove the dragged cards is done AFTER a non-transparent child
window
(the 'moving_cards_window') is plopped on top of it (all of the exercise done
drag_begin() to get the card images and carefully draw them on a Cairo
surface
that becomes the windows background has only one effect - to mark the exact
area where the cards are as being non-transparent - the generated background
pattern is never actually used :P ).
Not being a great expert at GDK, I used the first solution that I could find:
call begin_paint and immediately end_paint, which did exactly what was wanted
- to draw the Cairo background associated with the window.

best regards,

-- Leo





This patch restores the broken behavior of 'undo' which should call give-status-message
as defined by the game (and not the empty one defined in api.scm itself).

(c) 2015 snarfbeest
diff -ur --exclude help --exclude po aisleriot-3.10.2/games/api.scm ar-lk/games/api.scm
--- aisleriot-3.10.2/games/api.scm      2013-06-08 21:22:41.000000000 +0300
+++ ar-lk/games/api.scm 2015-02-14 14:29:48.117060306 +0200
@@ -760,8 +760,10 @@
        (set-cards! slot-id (car card-positions))
        (return-cards (cdr card-positions) (+ 1 slot-id))))
 
-(define-public (give-status-message)
-  #t)
+(define (give-status-message)
+  (if (module-variable (resolve-module '(guile-user)) 'give-status-message)
+    ( (@@ (guile-user) give-status-message) )
+    (#t)) )
 
 (define-public (eval-move move)
   (return-cards (caddr move) 0)

Add a simple invalidate-handler that redraws the the moving cards window with its
pre-set background.
It appears that the change to 'native window' added to work around some GDK bug
also causes GDK to give up entirely the responsibility of painting the window.
Therefore without a handler, the window is created in 'invalid' state and is never
actually painted (it simply retains the card images from the board when dragging
begins), making the card images messed up if they are dragged partially out of
the parent window's boundary.

(c) 2015 snarfbeest
diff -ur --exclude help --exclude po aisleriot-3.10.2/src/board-noclutter.c ar-lk/src/board-noclutter.c
--- aisleriot-3.10.2/src/board-noclutter.c      2013-11-05 15:50:21.000000000 +0200
+++ ar-lk/src/board-noclutter.c 2015-02-14 13:42:35.624992448 +0200
@@ -817,6 +817,13 @@
 }
 
 static void
+moving_cards_redraw(GdkWindow *window, cairo_region_t *region)
+{
+gdk_window_begin_paint_region(window,region);
+gdk_window_end_paint(window);
+}
+
+static void
 drag_begin (AisleriotBoard *board)
 {
   AisleriotBoardPrivate *priv = board->priv;
@@ -897,6 +904,7 @@
 
   /* FIXME: workaround for gtk+ bug #702951 */
   gdk_window_ensure_native (priv->moving_cards_window);
+  gdk_window_set_invalidate_handler(priv->moving_cards_window,moving_cards_redraw);
 
   surface = gdk_window_create_similar_surface (priv->moving_cards_window, CAIRO_CONTENT_COLOR_ALPHA,
                                                width, height);