[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

URL: Summary: [patch] use unions to save the game settings Project: Freeciv Submitted by: syntron Submitted on: Sonntag 28.06.2009 um 22:07 Category: general Severity: 2 - Mino

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #1, bug #13810 (project freeciv): Why not using the union like: union { struct { bool *value; const bool default_value; const bool_validate_func_t validate; } boolean; struct { int *value; const int default_value; const int min_value; const int

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Update of bug #13810 (project freeciv): Assigned to:None => mbook ___ Reply to this item at: ___ Message sen

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #2, bug #13810 (project freeciv): The "*(op->value.bval)" and similar expressions would make good candidates for wrapper macros, so that the internal field names can be easily updated in the future if needed. Actually, probably static inline wrapper functions would be best, e.g

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #3, bug #13810 (project freeciv): > It looks really better and understandable to me. More > over, it should take the minimal size of the whole > structure, and not an addition of all of them. I agree with pepeto, the "union of structs" would probably make more sense, since what

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #4, bug #13810 (project freeciv): > static inline bool setting_boolval(conct struct setting_s *op) That should be "const" of course. I'm also not sure about the naming scheme, since with the full "setting_" prefix this makes it a little awkward to use in expressions. Perhaps

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #5, bug #13810 (project freeciv): new patch on top of the last one (depends on gna13805): update union in struct settings_s + wrapper functions * new union in struct settings_s by pepeto * idea for wrapper functions by mbook I also added a patch which includes both changesets

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #6, bug #13810 (project freeciv): version 3: - on top of gna13805 - all changes from this ticket (gna13810) included - summery: * remove all dependencies on settings_s in all files but settings.c * the struct definition uses an union * rename settings_s to setting If we tru

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #7, bug #13810 (project freeciv): version 4; changes: - rename op_* to pset_* (pset = pointer to struct setting) (file #6104) ___ Additional Item Attachment: File name: version4-gna13810.patch.diff Size:41 KB __

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #8, bug #13810 (project freeciv): I meant "pset" only for the variable names; functions structs, etc. should have "setting". The variable name should usually give some hint as to what it stands for. So "cmd", when the type is "struct setting *", would not be a good variable name

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #9, bug #13810 (project freeciv): > _(pset_short_help(i)) Why not put the _() in setting_short_help() and write in the comment header that it returns the translated version? This goes for all such dupicated expressions. Why do you use the setting number as first argument to al

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #10, bug #13810 (project freeciv): > Why do you use the setting number as first argument > to almost every setting function? The only function > that takes a setting id should be setting_by_number(). > > > for (i = 0; i < SETTINGS_NUM; i++) { This way I can use pset_scate

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #11, bug #13810 (project freeciv): > This way I can use > > pset_scategory(i) > > else I would have > > pset_scategory(setting_by_number(i)) There is an example of how to create an iteration macro when the array pointer is not available in the header in the patch in bug #

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #12, bug #13810 (project freeciv): > Sorry if something was unclear beforehand. > Unfortunately I will not be able to explain so much > now as I will be trying to finish the work for 2.2. Thanks for the help! I did learn something in the discussions and the last thing I did

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Follow-up Comment #13, bug #13810 (project freeciv): a new version can be found in gna 13867 ___ Reply to this item at: ___ Nachricht geschickt von/durch Gna!

[Freeciv-Dev] [bug #13810] [patch] use unions to save the game settings

Update of bug #13810 (project freeciv): Status:None => Duplicate Assigned to: mbook => cazfi Open/Closed:Open => Closed