[Freeciv-Dev] [patch #1203] move ruleset option illness into its own section
Update of patch #1203 (project freeciv): Status:None = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #6, patch #1203 (project freeciv): 1) I plan to commit original move illness into its own section patch. I will attach a diff against the current trunk to this ticket 2) Sorting values in game.ruleset and adding default values to default ruleset is good (as it is often used as ruleset format documentation). Adding default values to civ1 civ2 rulesets does not make similar sense and only adds maintenance work if default values ever change. I have created an updated patch which only sorts the game.ruleset files but does not change their content. I did test it using less game.ruleset | grep -v ^; | sort -bfu game.ruleset-content for both versions and compared the files. 3) I'm against secfile_lookup_int_default_min_max() that would directly exit() the server. We should change ruleset loading failure to act nicely in general and proposed secfile_lookup_int_default_min_max() goes to opposite direction. It's even worse than rest of the current ruleset loading code in that it doesn't send failure messages to client that spawned the server. I created a changed version which uses the min/max value if the ruleset setting is out of range. It is reported as LOG_ERROR. How do you propose the functions in registry.c should be changed? Add an extra argument 'char *err_msg' to _each_ of these functions which is NULL if there is no error, else it contains the error message? 4) I have not checked it closely but from what you say it seems like removal of GAME_DEFAULT_SLOW_INVASIONS makes sense. I will send a bug ticket, which only includes this change. (file #6472) ___ Additional Item Attachment: File name: 0001-move-ruleset-option-illness-into-its-own-section.patch Size:6 KB ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #5, patch #1203 (project freeciv): Please submit independent changes as separate tickets in the future. This ticket has currently strange mixture of good things and things I'm against. 1) I plan to commit original move illness into its own section patch. 2) Sorting values in game.ruleset and adding default values to default ruleset is good (as it is often used as ruleset format documentation). Adding default values to civ1 civ2 rulesets does not make similar sense and only adds maintenance work if default values ever change. 3) I'm against secfile_lookup_int_default_min_max() that would directly exit() the server. We should change ruleset loading failure to act nicely in general and proposed secfile_lookup_int_default_min_max() goes to opposite direction. It's even worse than rest of the current ruleset loading code in that it doesn't send failure messages to client that spawned the server. 4) I have not checked it closely but from what you say it seems like removal of GAME_DEFAULT_SLOW_INVASIONS makes sense. ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #3, patch #1203 (project freeciv): as mentioned in the last comment I did * insert the function secfile_lookup_int_default_min_max() * added all ruleset options as constants (default/min/max) * use the function to load them * cleanup of the game.ruleset files * move illness to its own section I hope the limits are chosen, that old files work without problems ... (file #6301) ___ Additional Item Attachment: File name: cleanup_of_ruleset_c.patch Size:51 KB ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #4, patch #1203 (project freeciv): update version * delete GAME_DEFAULT_SLOW_INVASIONS from game.c as it is initialised by the ruleset (file #6302) ___ Additional Item Attachment: File name: version2-cleanup_of_ruleset_c.patch Size:51 KB ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #2, patch #1203 (project freeciv): Should the hard coded numbers in ruleset.c be changed to constants defined in game.h? At the moment it is a mix out of numbers and constants. Furthermore, perhaps a function secfile_lookup_int_default_min_max() is needed? It would be equal to secfile_lookup_int_default() but includes a check for minimum / maximum allowed values. Such a check is included for some values but it could be a sanity check for most values as they should be = 0 (and = big value). ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
URL: http://gna.org/patch/?1203 Summary: move ruleset option illness into its own section Project: Freeciv Submitted by: syntron Submitted on: Montag 27.07.2009 um 20:26 Category: general Priority: 3 - Low Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any ___ Details: see summary the section 'civstyle' in game.ruleset is a little crowed - move illness option into their own section ___ File Attachments: --- Date: Montag 27.07.2009 um 20:26 Name: 0001-move-ruleset-option-illness-into-its-own-section.patch Size: 6kB By: syntron http://gna.org/patch/download.php?file_id=6275 ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Update of patch #1203 (project freeciv): Assigned to:None = cazfi ___ Reply to this item at: http://gna.org/patch/?1203 ___ 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] [patch #1203] move ruleset option illness into its own section
Follow-up Comment #1, patch #1203 (project freeciv): I did a complete cleanup of the game.ruleset files. The content is not changed, only sorted and missing default entries are added. (file #6276) ___ Additional Item Attachment: File name: cleanup_game_ruleset_files.patch Size:19 KB ___ Reply to this item at: http://gna.org/patch/?1203 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
