[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Update of bug #13867 (project freeciv):

  Status:None => Fixed  
 Open/Closed:Open => Closed 


___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Update of bug #13867 (project freeciv):

 Assigned to:   cazfi => pepeto 
 Planned Release:   2.3.0 => 2.2.0  

___

Follow-up Comment #15:

Updated version against current svn.


(file #7176)
___

Additional Item Attachment:

File name: trunk_S2_2_setting_wrappers2.diff Size:50 KB


___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Follow-up Comment #14, bug #13867 (project freeciv):

This include patch bug #13872.

___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Follow-up Comment #13, bug #13867 (project freeciv):

Of course, it interests me (because I'm doing similar changes in client
options).  But now, the most important question is, will Marko come back
soon?  May I steal him tickets?


___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #12, bug #13867 (project freeciv):

looks good! Are the following patches also of interest at the moment? I will
try to find the time to check the next patches in this set ...

___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Follow-up Comment #11, bug #13867 (project freeciv):

Taking in account all comments posted there.


(file #6857)
___

Additional Item Attachment:

File name: trunk_S2_2_setting_wrappers.diff Size:49 KB


___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[pepeto]

Follow-up Comment #10, bug #13867 (project freeciv):

> setting_to_client() is missing function header. We are not adding new
functions without proper headers.

That's right, a short description would be good.

Also, after reading this patch, I think that translator work could be
simplified.  Very small single translated words can be included in the
format.  Example:
_("Option: %s  -  %s"), setting_name(pset), _(setting_short_help(pset)
could be:
"%s %s  -  %s", _("Option:"), setting_name(pset), _(setting_short_help(pset)
then the translation job would be already done for:
_("Option: %s"), setting_name(pset)
which could be:
"%s %s", _("Option:"), setting_name(pset)
I think it could be done for all prefixes.  But do not to it for long
sentences.

I don't see why do you return the pointer to the functions.  If you make a
function interface like you did, it should be made more like an
encapsulation.  So it should be 2 function different:
bool setting_get_bval(const struct setting *pset);
void setting_set_bval(struct setting *pset, bool val);

Maybe, moves the definition of the setting struct in the .c file, to make the
type opaque, to emphasize the OOP design.


___

Reply to this item at:

  

___
  Message posté via/par Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Marko Lindqvist]

Follow-up Comment #9, bug #13867 (project freeciv):

setting_to_client() is missing function header. We are not adding new
functions without proper headers.

___

Reply to this item at:

  

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #8, bug #13867 (project freeciv):

rebased version of the patch:

get game settings via wrapper functions

* cleanup naming:
  - definition: struct setting {};
  - data: struct setting settings[];
  - pointer: struct setting *pset;
  - functions: setting_*()
* remove 'extern' flag from settings
* define (and use) iterating macro for settings

(file #6681)
___

Additional Item Attachment:

File name: 0001-get-game-settings-via-wrapper-functions.diff Size:40 KB


___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Marko Lindqvist]

Follow-up Comment #7, bug #13867 (project freeciv):

This patch does not compile when applied to current trunk.

___

Reply to this item at:

  

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Marko Lindqvist]

Update of bug #13867 (project freeciv):

 Assigned to:None => cazfi  
 Planned Release:None => 2.3.0  


___

Reply to this item at:

  

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #6, bug #13867 (project freeciv):

> [Still... it's unfortunate that Gna doesn't attach
> patches to the mailed copy of the ticket like was
> the case with bugs filed using RT.]

Yes, this would help. I don't know if it is possible?

___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #5, bug #13867 (project freeciv):

updated patch

changes:
- add more function comments

(file #6190)
___

Additional Item Attachment:

File name: 0001-get-game-settings-via-wrapper-functions.patch Size:39 KB


___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[John Keller]

Follow-up Comment #4, bug #13867 (project freeciv):

Ah, oops - my apologies for the noise.

I really should have read the patch before sounding off (since I clearly
misunderstood the translation tags as being used for static text rather than
calculated text). :-p

[Still... it's unfortunate that Gna doesn't attach patches to the mailed copy
of the ticket like was the case with bugs filed using RT.]

___

Reply to this item at:

  

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #3, bug #13867 (project freeciv):

the idea was to switch from a translation after the
wrapper function, i.e.

_(setting_short_help(pset))

to a structure which does the translation within the
wrapper functions, i.e.

const char *setting_short_help(const struct setting *pset)
{
  return _(pset->short_help);
}

but this is not (easily) possible as both - the
untranslated as well as the translated - text
is needed. 


___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[John Keller]

Follow-up Comment #2, bug #13867 (project freeciv):

> Using '_()' within the setting_*() functions is not
> done because the some callers need the untranslated
> string (see stdinhand.c:send_server_setting()). 

Wouldn't it be better, then, to use N_() instead of simply dropping _() ?

This change would still pull the string out for translation (ensuring that
these uses are included in the list of references), but wouldn't translate
the string itself. Look around the code for other uses to see examples,
that'll explain better than I could.

Maybe I'm wrong, or this is just a style choice, but thought I'd point that
possibility out anyway...

___

Reply to this item at:

  

___
  Message sent via/by Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

Follow-up Comment #1, bug #13867 (project freeciv):

loading the settings from the ruleset is problematic. If
the same ruleset is requested a second time, the settings
are not loaded (patch 6 & 7). I will look into this ...

___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

[Freeciv-Dev] [bug #13867] [patch 01/07] get game settings via wrapper functions

[Matthias Pfafferodt]

URL:
  

 Summary: [patch 01/07] get game settings via wrapper
functions
 Project: Freeciv
Submitted by: syntron
Submitted on: Sonntag 05.07.2009 um 22:15
Category: general
Severity: 2 - Minor
Priority: 5 - Normal
  Status: None
 Assigned to: None
Originator Email: 
 Open/Closed: Open
 Discussion Lock: Any
 Release: 
Operating System: None

___

Details:

get game settings via wrapper functions

* cleanup naming:
  - definition: struct setting {};
  - data: struct setting settings[];
  - pointer: struct setting *pset;
  - functions: setting_*()
* remove 'extern' flag from settings
* define (and use) iterating macro for settings

Using '_()' within the setting_*() functions is not
done because the some callers need the untranslated
string (see stdinhand.c:send_server_setting()).

This patch serie includes the following patches:

[patch 01/07] get game settings via wrapper functions
[patch 02/07] use an union in the struct setting
[patch 03/07] simplify validation functions
[patch 04/07] extra functions to check and update settings
[patch 05/07] add settting action callback function
[patch 06/07] move sset_is_to_client() to settings.c
[patch 07/07] load game settings from ruleset file

It supersedes gna13621, gna13805, gna13810

Each step results in a working freeciv version if all
previous patches are applied.



___

File Attachments:


---
Date: Sonntag 05.07.2009 um 22:15  Name:
01-get-game-settings-via-wrapper-functions.patch  Size: 39kB   By: syntron



___

Reply to this item at:

  

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev