[crossfire] [PATCH] Fix keybinding bug when connected to server-1.12

2013-11-04 Thread Arvid Brodin
As the subject says.

The patch should apply to client trunk rev 19091.

The fix is in three parts:

* If no character name is given (is NULL), don't load or save 
  character-specific keys file.
* Load key bindings directly on startup, when name is NULL.
* If playing on a server with a login process, set the 
  character name and load key bindings again.


Also, please do not modify the patches when applying them since 
it messes up any patches that I have queued. If something should 
be changed, tell me what and I'll generate a new one.

Thanks,
Arvid
From 72b2d4af20da13b00753119ada19016c9209f6fa Mon Sep 17 00:00:00 2001
From: Arvid Brodin arv...@kth.se
Date: Tue, 5 Nov 2013 00:15:14 +0100
Subject: [PATCH] Fix keybinding bug when connected to server-1.12.


Signed-off-by: Arvid Brodin arv...@kth.se
---
 gtk-v2/src/account.c |  6 +-
 gtk-v2/src/create_char.c |  6 +-
 gtk-v2/src/gtk2proto.h   |  2 +-
 gtk-v2/src/keys.c| 38 --
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/gtk-v2/src/account.c b/gtk-v2/src/account.c
index 4b1b598..02930ed 100644
--- a/gtk-v2/src/account.c
+++ b/gtk-v2/src/account.c
@@ -503,11 +503,7 @@ static void play_character(const char *name)
 SockList_AddString(sl, name);
 SockList_Send(sl, csocket.fd);
 
-if (cpl.name) {
-free(cpl.name);
-}
-cpl.name = strdup(name);
-keybindings_init();
+keybindings_init(name);
 }
 
 /**
diff --git a/gtk-v2/src/create_char.c b/gtk-v2/src/create_char.c
index ebca675..735107b 100644
--- a/gtk-v2/src/create_char.c
+++ b/gtk-v2/src/create_char.c
@@ -357,11 +357,7 @@ static void send_create_player_to_server()
 
 SockList_Send(sl, csocket.fd);
 
-if (cpl.name) {
-free(cpl.name);
-}
-cpl.name = strdup(char_name);
-keybindings_init();
+keybindings_init(char_name);
 }
 
 
diff --git a/gtk-v2/src/gtk2proto.h b/gtk-v2/src/gtk2proto.h
index 0e894a5..e87c54f 100644
--- a/gtk-v2/src/gtk2proto.h
+++ b/gtk-v2/src/gtk2proto.h
@@ -112,7 +112,7 @@ extern void animate_inventory(void);
 extern void animate_look(void);
 extern void inventory_tick(void);
 /* keys.c */
-extern void keybindings_init();
+extern void keybindings_init(const char *character_name);
 extern void keys_init(GtkWidget *window_root);
 extern void bind_key(char *params);
 extern void unbind_key(const char *params);
diff --git a/gtk-v2/src/keys.c b/gtk-v2/src/keys.c
index 3fdcd9b..200d69d 100644
--- a/gtk-v2/src/keys.c
+++ b/gtk-v2/src/keys.c
@@ -501,7 +501,7 @@ static int parse_keys_file(char *filename)
  * from main() as part of the client start up. The function is common to both
  * the x11 and gdk clients.
  */
-void keybindings_init()
+void keybindings_init(const char *character_name)
 {
 int i;
 char buf[BIG_BUF];
@@ -540,6 +540,18 @@ void keybindings_init()
 }
 
 /*
+ * If we were supplied with a character name, store it so that we
+ * can load and save a character-specific keys file.
+ */
+if (cpl.name) {
+free(cpl.name);
+cpl.name = NULL;
+}
+if (character_name) {
+cpl.name = strdup(character_name);
+}
+
+/*
  * We now try to load the keybindings.  First place to look is the users
  * home directory, ~/.crossfire/keys.  Using a directory seems like a
  * good idea, in the future, additional stuff may be stored.
@@ -549,10 +561,12 @@ void keybindings_init()
  * in character files to this format, all that needs to be done is remove
  * the 'key ' at the start of each line.
  */
-
-/* Try the character-specific keys file */
-snprintf(buf, sizeof(buf), %s/.crossfire/%s.keys, getenv(HOME), cpl.name);
-res = parse_keys_file(buf);
+res = -1;
+if (cpl.name) {
+/* Try the character-specific keys file */
+snprintf(buf, sizeof(buf), %s/.crossfire/%s.keys, getenv(HOME), cpl.name);
+res = parse_keys_file(buf);
+}
 if (res  0) {
 /* Try the user-specific keys file */
 snprintf(buf, sizeof(buf), %s/.crossfire/keys, getenv(HOME));
@@ -696,6 +710,14 @@ void keys_init(GtkWidget *window_root)
 for (i = 0; i  KEYHASH; i++) {
 keys[i] = NULL;
 }
+
+/*
+ * Old servers (e.g. 1.12) starts game play without a login
+ * process. We can't get the character name on such a server, so
+ * load default (or user-specific) key bindings here in case we
+ * don't get the character-specific one later.
+ */
+keybindings_init(NULL);
 }
 
 /**
@@ -1197,7 +1219,11 @@ static void save_keys(void)
 int i;
 FILE *fp;
 
-snprintf(buf, sizeof(buf), %s/.crossfire/%s.keys, getenv(HOME), cpl.name);
+if (cpl.name) {
+snprintf(buf, sizeof(buf), %s/.crossfire/%s.keys, getenv(HOME), cpl.name);
+} else {
+snprintf(buf, sizeof(buf), %s/.crossfire/keys, getenv(HOME));
+}
 CONVERT_FILESPEC_TO_OS_FORMAT(buf);
 LOG(LOG_WARNING, gtk-v2::save_keys, Saving 

Re: [crossfire] [PATCH] Fix keybinding bug when connected to server-1.12

2013-11-04 Thread Kevin Zheng
On 11/04/2013 17:41, Arvid Brodin wrote:
 As the subject says.
 
 The patch should apply to client trunk rev 19091.

Committed in r19094, thanks!

 Also, please do not modify the patches when applying them since 
 it messes up any patches that I have queued. If something should 
 be changed, tell me what and I'll generate a new one.

Point taken.

Thanks,
Kevin Zheng
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire


Re: [crossfire] [PATCH 2/2] Character-specific keybinding files

2013-11-04 Thread Mark Wedel

On 11/ 4/13 05:34 AM, Arvid Brodin wrote:

On 2013-11-04 07:38, Mark Wedel wrote:

On 11/ 3/13 03:03 PM, Kevin Zheng wrote:

On 11/03/2013 13:36, Mark Wedel wrote:

   Just a question - is there any way to set up 'global' keybindings (eg,
those that apply to all characters)?


This does seem useful. One possible idea to throw around is to make the
key bindings window tabbed, one for per-character and one for global. If
there's a conflict, prefer the per-character binding.

Of course, this is easier said than done.


  True on both accounts.  Having per character keybindings take preference over 
global ones would be the correct approach.  However, in an ideal world, both 
are visible, so one could easily see the conflict and make adjustments.

  Note that it appears this patch introduces a new bug, as posted on the forums:

Posted: Sun Nov 03, 2013 6:51 pm
Post subject : Compatibility with new character-specific keybindings?
 I updated my trunk client to the newest version (r19093), which has the 
character-specific keybindings as a feature.
When I logged into Metalforge with the new client, absolutely no keybindings 
work (not even the default ones). Any keybindings I try to create go to a 
(null).keys file and do not persist to another login.
When I log into netarbeiter (1.70.0), my keybindings work fine.

Does this mean the newest trunk client is incompatible with the 1.12 branch? Or 
am I doing something wrong?
--

  I suspect the issue is that metalforge is running old code, which predates 
the new character login logic.  As such, the client never has a character name 
of which to load keybindings for (and I haven't looked, but depending on where 
the load actually happens, perhaps never load them at all.


The names of the characters are returned from the server as a response to
AccountPlayersCmd (if I understand correctly), then stored in
character_choose_window until play is started with a character, whose name
is then stored in cpl.name and is used for loading and saving the keys
file. If the name of the chosen character is NULL, that would be passed to
strdup().

No check is made since I assumed that it is not possible to start game play
without having a list of characters to choose from.

I just tried to login at metalforge and the game is instantly started with
the character dwarf the dwarf... I.e. the whole login process is skipped?

Can anyone describe to me how this works? (I can't test any more now, need
to get to work...)



 metalforge is running an older version of the server, so yes, all of that is 
skipped, and the server does not support the protocol command for the client to 
get a list of characters available to the player.


 So what you see is how it works - you enter the character name, and then 
password, and that is the login process.


 Now a fair question to ask is whether the client should support old server 
versions or not - IMO, if having support is not hard, it should, but if making 
it work would be really difficult, then probably not.  Although, in that case, 
the client should see that limitation and print a message saying 'server to old 
- use an older client on that server' or the like.


___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire


Re: [crossfire] Use of svn:externals in client code on SVN

2013-11-04 Thread Mark Wedel

On 11/ 4/13 04:41 PM, Kevin Zheng wrote:

Hi there,

Crossfire currently uses svn:externals to sync protocol definition
headers between the server and GTKv2 client. While this has its merits,
overall I believe it is confusing, cumbersome, and harmful.

 From the SVN Book [1]: An externals definition is a mapping of a local
directory to the URL—and ideally a particular revision—of a versioned
directory. In Subversion, you declare externals definitions in groups
using the svn:externals property.

My guess is that the original intention of using them was to ensure that
the client protocol headers would always remain in sync with those in
the server. However, here are some issues:

  - The svn:externals property must still be modified manually
  - Copy/paste/merge is faster, less confusing, and works outside of SVN
  - Client sources locked to server, JXClient proves unnecessary

Further, there are several disadvantages to using them as well:

  - It only works with SVN - hinders future migration (if ever)
  - Confusing for people not familiar with SVN
  - Slow, try running `svn up` from the root and wait
  - Breaks `git svn`, which pacifies the impatient Git users

I propose that we go back to manually tracking the protocol headers
between the client and server. They are not updated that often, anyways,
and still need updating with the use of svn:externals. We can continue
using svn:externals in /latest, etc.

[1] http://svnbook.red-bean.com/en/1.7/svn.advanced.externals.html


 That is fine by me.  And before the externals was set up, my method of sync 
wast just a cp from one directory to the other.


___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire