Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-11-03 Thread Mark Wedel

On 11/ 2/13 08:14 PM, Kevin Zheng wrote:

On 11/02/2013 11:04, Arvid Brodin wrote:



I noticed the gtk client code is split between the folders common/
and gtk-v2/. I found references to something called the x11 client
in the code, and I assumed that that was an old client that had since
been removed. It was apparently called cfclient, later renamed
crossfire-client-x11. If this has been removed, perhaps it would be a
good idea to merge the common/ folder into the gtk-v2/ folder?


You've probably noticed that the sources in common/ are built into a
shared library. The intention was to share some code between the several
different versions of the client (X11, SDL, OpenGL, GTKv1, GTKv2). Now,
it seems a little unnecessary (unless someone decides to bring back
another client).


 But to me, there is very little reason to merge them - the common area vs the 
specific graphic area are pretty well abstracted apart.


 And while right now there might only be one graphics client that uses it, if 
they get merged, that pretty much would mean that would always be the case.


 In terms of history, early version of the client only had X11 client support - 
the fact it was separated made it possible to have different GUI clients at 
different times (gtkv1 vs gtkv2, and I think there was a gnome client at some 
point).


 Now whether there will ever be new graphics system ever added the the C client 
(vs java), who knows, but off the top of my head, I could think of several 
platforms where a native GUI client could be written, and who knows what changes 
may show up even on the unix desktops which might bring forth a new client with 
new toolkit.


 If there are specific issues or problems trying to be addressed by merging 
them together, that would be good to hear (and perhaps could be fixed without a 
merge).  But just merging them for the sake of merging may not be good long term.


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


Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-11-02 Thread Arvid Brodin
On 2013-11-01 04:54, David Hurst wrote:
 I'd be of the view, if it isn't reducing a functionality but
 re-implementing it in a more useful way (be it more flexible, faster,
 or any other good reason) then it has my support.

This could better be described as removing an arcane feature to simplify 
the code and to make room for other improvements in the future. (Do you 
use the feature of rebinding the Run, Shift, Alt or Meta keys?)

 Perhaps another perspective is that if you change it, and things
 don't work out, it is a lot easier to backtrack and try and different
 approach, than it is to have abandoned the idea before anyone got to
 play with it and never know what we were missing in the first place
 :).
 
 I would mention that at the moment windows users are really limited
 to the jxclient, so do try and make a change that can be implemented
 on both the gtk and jxclient.

That's something of a tall order. :) It's a lot of work learning how
_one_ client works; learning two clients and then trying do do only
changes that benefit both (and doesn't break any) of them could be
very limiting. (Is the Windows compatibility broken on the gtk client?)

Are there any code dependencies between jxclient and the gtk client? 

I noticed the gtk client code is split between the folders common/
and gtk-v2/. I found references to something called the x11 client
in the code, and I assumed that that was an old client that had since
been removed. It was apparently called cfclient, later renamed 
crossfire-client-x11. If this has been removed, perhaps it would be a 
good idea to merge the common/ folder into the gtk-v2/ folder?

(BTW, I've been working exclusively on the code in 
https://svn.code.sf.net/p/crossfire/code/client/trunk)



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


Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-11-02 Thread Kevin Zheng
On 11/02/2013 11:04, Arvid Brodin wrote:
 That's something of a tall order. :) It's a lot of work learning how
 _one_ client works; learning two clients and then trying do do only
 changes that benefit both (and doesn't break any) of them could be
 very limiting. (Is the Windows compatibility broken on the gtk client?)

Maintaining the Windows port of the GTKv2 client is difficult. Nicholas
tried to build a Windows version from trunk, with limited success.

 Are there any code dependencies between jxclient and the gtk client? 

No, not that I am aware of.

 I noticed the gtk client code is split between the folders common/
 and gtk-v2/. I found references to something called the x11 client
 in the code, and I assumed that that was an old client that had since
 been removed. It was apparently called cfclient, later renamed 
 crossfire-client-x11. If this has been removed, perhaps it would be a 
 good idea to merge the common/ folder into the gtk-v2/ folder?

You've probably noticed that the sources in common/ are built into a
shared library. The intention was to share some code between the several
different versions of the client (X11, SDL, OpenGL, GTKv1, GTKv2). Now,
it seems a little unnecessary (unless someone decides to bring back
another client).

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


Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-10-31 Thread Kevin Zheng
Hi there,

On 10/30/2013 20:06, Arvid Brodin wrote:
 As a new player, I found this to be a usability problem. Everyone knows what
 the Control key is, but what's the Run key? I think the reason Shift and Ctrl
 were called Fire and Run is that it is possible to rebind them - i.e. if you 
 want some other key to act as the Run modifier, you could
 
 'bind runkey1

I wasn't aware that this feature even existed. If it isn't present in
the JXClient, I'd say it's safe to take it out.

 - and press a key to act as the Run modifier. (Actually I'm not even sure this
 worked; I never tried it - and I'm pretty sure it requires the run_on state 
 to 
 be kept in the server, since key repeats won't work if you bind Run to, say, 
 'r', and press that after the direction key... hmm. So it might be in the way
 of better handling of repeating keys.)

Probably not, repeating keys should be handled more elegantly.

 I realize this change might not be liked by everyone, so feel free to shout 
 out
 in that case (RFC = Request For Comments). But it did seem like a pretty 
 unique
 and not very well advertised feature, so I chanced that it could be removed.

You have my vote to take it out. Input from others would be helpful.

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


Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-10-31 Thread David Hurst
I'd be of the view, if it isn't reducing a functionality but
re-implementing it in a more useful way (be it more flexible, faster, or
any other good reason) then it has my support.
Perhaps another perspective is that if you change it, and things don't work
out, it is a lot easier to backtrack and try and different approach, than
it is to have abandoned the idea before anyone got to play with it and
never know what we were missing in the first place :).

I would mention that at the moment windows users are really limited to the
jxclient, so do try and make a change that can be implemented on both the
gtk and jxclient.

Saru


On Fri, Nov 1, 2013 at 10:24 AM, Kevin Zheng kevinz5...@gmail.com wrote:

 Hi there,

 On 10/30/2013 20:06, Arvid Brodin wrote:
  As a new player, I found this to be a usability problem. Everyone knows
 what
  the Control key is, but what's the Run key? I think the reason Shift and
 Ctrl
  were called Fire and Run is that it is possible to rebind them - i.e. if
 you
  want some other key to act as the Run modifier, you could
 
  'bind runkey1

 I wasn't aware that this feature even existed. If it isn't present in
 the JXClient, I'd say it's safe to take it out.

  - and press a key to act as the Run modifier. (Actually I'm not even
 sure this
  worked; I never tried it - and I'm pretty sure it requires the run_on
 state to
  be kept in the server, since key repeats won't work if you bind Run to,
 say,
  'r', and press that after the direction key... hmm. So it might be in
 the way
  of better handling of repeating keys.)

 Probably not, repeating keys should be handled more elegantly.

  I realize this change might not be liked by everyone, so feel free to
 shout out
  in that case (RFC = Request For Comments). But it did seem like a pretty
 unique
  and not very well advertised feature, so I chanced that it could be
 removed.

 You have my vote to take it out. Input from others would be helpful.

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

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


Re: [crossfire] [RFC 2/3] Misc keybinding fixes and changes

2013-10-30 Thread Arvid Brodin
On 2013-10-31 00:34, Kevin Zheng wrote:
 Due to a minor change (addition of braces in a 'for' loop) in the
 previous patch (RFC 1/3), the original patch no longer applies cleanly.
 
 I've attached a resolved patch that should apply correctly now. Please
 tell me if something doesn't look right. Sorry for the confusion.

I get this diff of keys.c after applying my patch #1 vs your patch #1:

--- git-client/gtk-v2/src/keys.c2013-10-31 01:57:39.109350292 +0100
+++ client-trunk/gtk-v2/src/keys.c  2013-10-31 01:51:03.551723009 +0100
@@ -186,11 +186,12 @@ static void insert_key(uint32 keysym, in
  * Try to find out if the command is a direction command.  If so, keep
  * track of this fact, so in fire or run mode, things work correctly.
  */
-for (i = 0; i  9; i++)
+for (i = 0; i  9; i++) {
 if (!strcmp(command, directions[i])) {
 direction = i;
 break;
 }
+}
 
 newkey-keysym = keysym;
 newkey-flags = flags;
@@ -763,7 +764,7 @@ static void parse_key(char key, uint32 k
 return;
 }
 if (key = '0'  key = '9') {
-cpl.count = cpl.count*10 + (key - '0');
+cpl.count = cpl.count * 10 + (key - '0');
 if (cpl.count  10) {
 cpl.count %= 10;
 }

It all looks good to me! Also, patch #3 applies with just some offset on top 
of the modified patches.

 
 On 10/28/2013 18:19, Arvid Brodin wrote:
 * Add Any key modifier type; New Any button in keybindings dialog. This 
 lets
 the user choose between having a key work regardless of modifier (Ctrl, Alt, 
 etc)
 key states, or to use the same key with different modifiers for different 
 bindings. How this worked before (All/Normal) was pretty unclear, even 
 in the
 code comments. (New feature/bug fix.)
 
 If Any is not checked, the key binding should retain its present
 behavior, correct? That is, the key only works when no modifiers are
 being pressed.

Correct.

 
 * Removed possibility to rebind Ctrl, Shift, Alt, Meta modifiers. This 
 feature
 is very unusual and breaks the normal design pattern of using the designated 
 keys as keyboard modifiers.
 
 I'm not sure I understand the original problem. Could you please explain
 the rebinding part?

As a new player, I found this to be a usability problem. Everyone knows what
the Control key is, but what's the Run key? I think the reason Shift and Ctrl
were called Fire and Run is that it is possible to rebind them - i.e. if you 
want some other key to act as the Run modifier, you could

'bind runkey1

- and press a key to act as the Run modifier. (Actually I'm not even sure this
worked; I never tried it - and I'm pretty sure it requires the run_on state to 
be kept in the server, since key repeats won't work if you bind Run to, say, 
'r', and press that after the direction key... hmm. So it might be in the way
of better handling of repeating keys.)

I realise this change might not be liked by everyone, so feel free to shout out
in that case (RFC = Request For Comments). But it did seem like a pretty unique
and not very well advertised feature, so I chanced that it could be removed.



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


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