Follow-up Comment #1, patch #1179 (project wesnoth):
I like this idea a lot (I would even extend so that a notification is sent at
start of turn), but the patch doesn't seem ready. Here are a few things I
noticed.
First, why are there all these changes to menu_events.cpp? I would have
thought the changes to game_display.cpp were sufficient. Am I missing
something?
Second, the feature should not depend on HAVE_LIBX11, it should depend on
HAVE_LIBNOTIFY or something. That way, X people that can't stand Gnome won't
have to install it; and Windows or MacOSX or whatever people who enjoy Gnome
can benefit from notifications too. Also the send_notification function
itself (and its call) should not be conditional, only its body should be.
That way people will be able to implement notifications that don't depend on
libnotifymm.
Third, some comments about the code. You can simplify the send_notification
function a bit: don't bother with the boolean window_is_active, just exit
early when the windows is active. You can also avoid the preprocessor games
with foreach by including libnotifymm earlier. And finally, the call to
send_notification should be later in the function, so that it benefits from
the error checking below.
Fourth, some patch issues. Take a look at it, you will notice quite a bit of
unrelated changes: adding some empty lines, protecting menu_events.cpp with
preprocessor checks, merging SDL_LIBS settings.
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?1179>
_______________________________________________
Message posté via/par Gna!
http://gna.org/
_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs
