Re: [Freeciv-Dev] (PR#39831) Training and combat
http://bugs.freeciv.org/Ticket/Display.html?id=39831 > "Joan Creus" <[EMAIL PROTECTED]> wrote: > >http://bugs.freeciv.org/Ticket/Display.html?id=39831 > > >I think you mean EFT_VETERAN_BUILD. That sounds right. >I would try to make it shorter, though. >I think your suggestion is great for the general manual, but here we are >dealing with the individual unit help. The original sentence ("may become >veteran throught training or combat") was definitely too short, but this is >probably too much. And we have to think that this will be read by complete >newbies. You don't want to bother them with rulesets. I see what you mean. I overlooked the fact that this text is for the unit help. >I see your point, though. Borrowing also from Randy's, how about: > >Will be built as a veteran in certain cities (see Barracks, and Sun Tzu). >May also become veteran if it defeats an enemy unit. > >I don't like the "certain" word, but it's a start. I think it's important to >refer them to other parts of the manual, where they will get the full >explanation, while keeping it short. Instead of "certain cities", how about 1) "cities with appropriate training facilities", 2) "properly equipped cities", or 3) "appropriately equipped cities"? -Eddie ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39830) 2.1.0 border expansion acquires destroyed city
http://bugs.freeciv.org/Ticket/Display.html?id=39830 > Per I. Mathisen wrote: > If you are thinking about the distinction between server view of the > world, and client views of the world (both contained within the > server), then yes, these view may be radically different due to fog of > war. However, borders operate only on the server view of the world, > and does not touch client views at all (which may be kind of odd when > you think about it, but it has to be this way, or things will become > really strange when players meet and they try to figure out where > their respective borders are). > And therein lies the rub. I think I've figured out the problem, and it's partly to do with the server giving out the vision information based on the server view. The server "knows" the city isn't there anymore. It expands the border, but doesn't update the vision. Probably, send_map_is_known_and_seen() needs to return TRUE when the tile is owned by the player. Easy enough. Meanwhile, the server "knows" the client "knows" the city is there. The border code currently has (at least) two potentially conflicting values: struct player *owner; /* Player owning this tile, or NULL. */ struct tile *owner_source; /* what makes it owned by owner */ The latter isn't actually pointing to the source, it's pointing to the tile of the source. That tile no longer really has the source on it in the server view, but has it in the client view! Moreover, the savegame map_load should run map_calculate_borders(). Is there a reason it was only done between turns? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39734) unittype translation need qualifier
http://bugs.freeciv.org/Ticket/Display.html?id=39734 > _("?unit:?unitclass:Land") Just one qualifier And we'll need some code, as this is likely to affect several things. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39731) support for appended ruler titles
http://bugs.freeciv.org/Ticket/Display.html?id=39731 > Sounds easier and more fun than the border bugs I'll look at it tonight! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39734) unittype translation need qualifier
http://bugs.freeciv.org/Ticket/Display.html?id=39734 > > [dmarks - Sun Sep 30 07:53:23 2007]: > > On 9/29/07, Egor Vyscrebentsov <[EMAIL PROTECTED]> wrote: > > > > http://bugs.freeciv.org/Ticket/Display.html?id=39734 > > > > > Good daytime! > > > > Unittype strings have contradictions with rulesets (for Helicopter and > > Nuclear.) This words seems to have different sense in unittype and > > units ruleset. (At least, in russian.) > > > > So I ask for using Q_() qualifiers for unittype strings. > > > > -- > > Thanks, evyscr > > > > > > Would be useful for Swedish too, so you have my support. > > ~Daniel > > > Untested patch against S2_2. ~Daniel unit_qualifiers.diff Description: Binary data ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39731) support for appended ruler titles
http://bugs.freeciv.org/Ticket/Display.html?id=39731 > > [wsimpson - Thu Sep 27 12:33:21 2007]: > > Daniel Markstedt wrote: > > On 9/27/07, Egor Vyscrebentsov <[EMAIL PROTECTED]> wrote: > >> So, what do you want? A function which will return string contains > >> leader name and title in proper order? > > > > Yes. After that, all clients have to be modified to use this function > > instead of manually prepending titles. > > > Should this be based on the language of the user? Or the language of > the title? > Both, I think. In English, the title "king" is prepended, while the Chinese translation "wang" is appended. At the same time, the Mongolian title "khan" is appended in both languages, i.e. "Kublai Khan". > A data driven design for the latter could be modifying the titles to > have a %s where the leader name is properly placed. That should help > with right to left languages, too. > Sounds like an excellent solution! Then we can define an English default, while translators are allowed to invert it if it's required in their own languages. > I'd also like to modify the data to pair leader name and sex. It's a > minor nuisance, but counting is a bit of an annoyance. > > > Maybe you could revive PR#18260... ~Daniel ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39565) NULL instead of valid impr_type in punittype->need_improvement
http://bugs.freeciv.org/Ticket/Display.html?id=39565 > As explained in PR#39385, this is exactly the kind of bug that PR#39553 was intended to catch. The old code didn't check for a valid improvement before using it. The fix was much easier than removing the useful log, or writing this report Your patch is reverted. + } else if (NULL == impr_req) { + CITY_LOG(LOG_DEBUG, pcity, "cannot build unit %s", + utype_rule_name(punittype)); ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39828) 2.1.0 crash shared vision in send_tile_info()
http://bugs.freeciv.org/Ticket/Display.html?id=39828 > This patch has two parts. The quick and dirty solution is to treat terrain just like resources: when the pointer is NULL, send -1. That fixes the server crash. - info.type = plrtile->terrain->index; + info.type = plrtile->terrain ? plrtile->terrain->index : -1; === But now you are sending bad data to the client! The second part is to add validity checking to the client. This is a good idea in any case. (Never trust network data.) This reveals that a single tile is bad. 1: handle_tile_info() unknown terrain (1,25). === For some historic reason, the enum known_type is in terrain.h (where it's never used). Move to tile.h (where is the value is returned). Adjusted various #includes to match. Also, add a few instances of tile_special_type_iterate() from S2_2. === Reminder, this only fixes the 2.1.0 shared vision, not the other bugs reported in the same ticket. Committed S2_1 revision 13916. Committed S2_2 revision 13917. Committed trunk revision 13918. Final S2_2 changes for posterity (a bit different from S2_1, same as trunk): Index: server/maphand.c === --- server/maphand.c(revision 13916) +++ server/maphand.c(working copy) @@ -614,17 +614,18 @@ conn_list_iterate(dest, pconn) { struct player *pplayer = pconn->player; -enum tile_special_type spe; if (!pplayer && !pconn->observer) { continue; } if (!pplayer || map_is_known_and_seen(ptile, pplayer, V_MAIN)) { info.known = TILE_KNOWN; - info.type = terrain_number(ptile->terrain); - for (spe = 0; spe < S_LAST; spe++) { + info.type = ptile->terrain ? terrain_number(ptile->terrain) : -1; + + tile_special_type_iterate(spe) { info.special[spe] = BV_ISSET(ptile->special, spe); - } + } tile_special_type_iterate_end; + info.resource = ptile->resource ? resource_number(ptile->resource) : -1; info.continent = ptile->continent; send_packet_tile_info(pconn, &info); @@ -633,19 +634,23 @@ struct player_tile *plrtile = map_get_player_tile(ptile, pplayer); info.known = TILE_KNOWN_FOGGED; - info.type = terrain_number(plrtile->terrain); - for (spe = 0; spe < S_LAST; spe++) { + info.type = plrtile->terrain ? terrain_number(plrtile->terrain) : -1; + + tile_special_type_iterate(spe) { info.special[spe] = BV_ISSET(plrtile->special, spe); - } + } tile_special_type_iterate_end; + info.resource = plrtile->resource ? resource_number(plrtile->resource) : -1; info.continent = ptile->continent; send_packet_tile_info(pconn, &info); } else if (send_unknown) { info.known = TILE_UNKNOWN; info.type = -1; - for (spe = 0; spe < S_LAST; spe++) { + + tile_special_type_iterate(spe) { info.special[spe] = FALSE; - } + } tile_special_type_iterate_end; + info.resource = -1; info.continent = 0; send_packet_tile_info(pconn, &info); Index: common/aicore/path_finding.h === --- common/aicore/path_finding.h(revision 13916) +++ common/aicore/path_finding.h(working copy) @@ -14,7 +14,7 @@ #define FC__PATH_FINDING_H #include "map.h" -#include "terrain.h" +#include "tile.h" #include "unit.h" #include "unittype.h" Index: common/tile.h === --- common/tile.h (revision 13916) +++ common/tile.h (working copy) @@ -20,6 +20,13 @@ #include "terrain.h" #include "unitlist.h" +/* network, order dependent */ +enum known_type { + TILE_UNKNOWN = 0, + TILE_KNOWN_FOGGED = 1, + TILE_KNOWN = 2, +}; + /* Convenience macro for accessing tile coordinates. This should only be * used for debugging. */ #define TILE_XY(ptile) ((ptile) ? (ptile)->x : -1), \ Index: common/terrain.c === --- common/terrain.c(revision 13916) +++ common/terrain.c(working copy) @@ -496,14 +496,13 @@ / enum tile_special_type find_special_by_rule_name(const char *name) { - enum tile_special_type i; - assert(ARRAY_SIZE(tile_special_type_names) == S_LAST); - for (i = 0; i < S_LAST; i++) { + + tile_special_type_iterate(i) { if (0 == strcmp(tile_special_type_names[i], name)) { return i; } - } + } tile_special_type_iterate_end; return S_LAST; } Index: common/terrain.h === --- common/terrain.h(revision 13916) +++ common/terrain.h(working copy) @@ -55,12 +55,10 @@ BV_DEFINE(bv_special, S_LAST_PLUS); -/* currently only used in edithand.c */ #define tile_special_type_iterate(special) \ {
[Freeciv-Dev] (PR#39836) The freeciv theme only overwrites the gtk theme only particular
http://bugs.freeciv.org/Ticket/Display.html?id=39836 > I use for my gtk-apps the Graphite Metal Theme. http://themes.freshmeat.net/projects/graphitemetal-gtk2 The civclient shows the Golden freeciv decoration only on tabs (Maps, Scientific, Cities,... as well as Travellers, News,...) and on the status part (Population, Year, Gold, Taxes) and on the field where the actual units are shown (E.g. if a field with a unit is focussed, then this unit is shown on the left side). All the other parts of the window decoration are overwritten by the selected gtk2-theme. I wrote the selected gtk-theme, because it uses pixmaps for decoration and not only colors. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39834) get_embassy_status function removed(#13567), but there is call to it in win32 client
http://bugs.freeciv.org/Ticket/Display.html?id=39834 > Call to get_embassy_status is left in gui-win32/plrdlg.c ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39831) Training and combat
http://bugs.freeciv.org/Ticket/Display.html?id=39831 > I think you mean EFT_VETERAN_BUILD. I would try to make it shorter, though. I think your suggestion is great for the general manual, but here we are dealing with the individual unit help. The original sentence ("may become veteran throught training or combat") was definitely too short, but this is probably too much. And we have to think that this will be read by complete newbies. You don't want to bother them with rulesets. I see your point, though. Borrowing also from Randy's, how about: Will be built as a veteran in certain cities (see Barracks, and Sun Tzu). May also become veteran if it defeats an enemy unit. I don't like the "certain" word, but it's a start. I think it's important to refer them to other parts of the manual, where they will get the full explanation, while keeping it short. Joan 2007/11/4, Eddie Anderson <[EMAIL PROTECTED]>: > > > http://bugs.freeciv.org/Ticket/Display.html?id=39831 > > > "Joan Creus" <[EMAIL PROTECTED]> wrote: > > > >http://bugs.freeciv.org/Ticket/Display.html?id=39831 > > > > >Good suggestion. I like it. Just one point: shouldn't it be "with > Barracks" > >instead of "with a Barracks"? I'm not a native English speaker, but it > >doesn't feel right. > > > >Can anybody improve on Randy's suggestion? > > I suggest some wording that refers to the name of the EFT > (effect) that having a Barracks or Sun Tzu causes. I can't remember > the name of it at the moment (I'll just call it 'upgrade' in this > email). > > The reason for doing so is that it is more portable - i.e. the > wording will be appropriate for rulesets that may use other > buildings or wonders that cause the upgrade effect. Consider this > wording: > > "Land units sometimes gain 'veteran' status by defeating an enemy > unit in combat. Also, land units can be created with 'veteran' > status if they are created in a city where 'upgrade' is in effect. > The 'upgrade' can be caused by buildings (e.g. Barracks), Wonders > (e.g. Sun Tzu), or other factors defined in the ruleset." > > Replace 'upgrade' with the name of the EFT and see how it > sounds. I still can't remember the name. Sorry. > > HTH. > > -Eddie > > > I think you mean EFT_VETERAN_BUILD. I would try to make it shorter, though. I think your suggestion is great for the general manual, but here we are dealing with the individual unit help. The original sentence ("may become veteran throught training or combat") was definitely too short, but this is probably too much. And we have to think that this will be read by complete newbies. You don't want to bother them with rulesets. I see your point, though. Borrowing also from Randy's, how about:Will be built as a veteran in certain cities (see Barracks, and Sun Tzu). May also become veteran if it defeats an enemy unit.I don't like the "certain" word, but it's a start. I think it's important to refer them to other parts of the manual, where they will get the full explanation, while keeping it short. Joan2007/11/4, Eddie Anderson <[EMAIL PROTECTED]>: http://bugs.freeciv.org/Ticket/Display.html?id=39831 >"Joan Creus" <[EMAIL PROTECTED] > wrote:>>http://bugs.freeciv.org/Ticket/Display.html?id=39831 >>>Good suggestion. I like it. Just one point: shouldn't it be "with Barracks" >instead of "with a Barracks"? I'm not a native English speaker, but it>doesn't feel right.>>Can anybody improve on Randy's suggestion?I suggest some wording that refers to the name of the EFT (effect) that having a Barracks or Sun Tzu causes. I can't rememberthe name of it at the moment (I'll just call it 'upgrade' in thisemail).The reason for doing so is that it is more portable - i.e. thewording will be appropriate for rulesets that may use otherbuildings or wonders that cause the upgrade effect. Consider thiswording:"Land units sometimes gain 'veteran' status by defeating an enemy unit in combat. Also, land units can be created with 'veteran'status if they are created in a city where 'upgrade' is in effect.The 'upgrade' can be caused by buildings (e.g. Barracks), Wonders (e.g. Sun Tzu), or other factors defined in the ruleset." Replace 'upgrade' with the name of the EFT and see how itsounds. I still can't remember the name. Sorry. HTH. -Eddie ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39835) BUG: 2.2.0-test fix PR#39562 and PR#39565
http://bugs.freeciv.org/Ticket/Display.html?id=39835 > Committed S2_2 revision 13914. Committed trunk revision 13915. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39835) BUG: 2.2.0-test fix PR#39562 and PR#39565
http://bugs.freeciv.org/Ticket/Display.html?id=39835 > Another problem found with PR#39562, this time on its stated purpose, better choice asserts. The assert() itself tests both ">= CT_NONE" and "!= CT_NONE" -- the previous version had commented out "!= CT_NONE". The assert() also doesn't use standardized *_count() access functions. Why the "do {} while(0)"? Oh well, I left that, it doesn't seem to do any harm === PR#39565 removed some error detection done by PR#39553 -- for building/improvements -- in a section labeled (long before PR#39553): /* This should never happen? */ Obviously, it was happening! But the logging didn't work, because the old code (also before PR#39553) used the value directly without checking for a valid improvement before calling the function: - } else if (can_build_improvement(pcity, - punittype->impr_requirement)) { + } else if (can_city_build_improvement_now(pcity, impr_req)) { Instead of finding and fixing the bug, remove the log?!?! That's counterproductive. It was a simple (NULL != impr_req) fix. Exactly the kind of bug that copious error detection was intended to find! There was more effort to stick 4 useless lines around the log and generate a PR And even more effort by me to find and undo the useless lines several months later. Maybe now we can discover the underlying problem(s). === So that this report isn't a completely painful waste of several hours time, added ai_choice_rule_name() to replace 3 switches introduced in PR#39827. Wish I'd thought of it before Changed the name of ai_unit_task_rule_name() to conform to practice. Check simple_ai_types[] for "A_NEVER != punittype->require_advance" in its update function, rather than at every use of its iterator. (Some didn't check.) Index: common/city.h === --- common/city.h (revision 13913) +++ common/city.h (working copy) @@ -166,13 +166,13 @@ #define ASSERT_CHOICE(c) \ do { \ if ((c).want > 0) { \ - assert((c).type >= 0 && (c).type < CT_LAST && (c).type != CT_NONE);\ + assert((c).type > CT_NONE && (c).type < CT_LAST); \ if ((c).type == CT_BUILDING) { \ int _iindex = improvement_index((c).value.building); \ -assert(_iindex >= 0 && _iindex < game.control.num_impr_types); \ +assert(_iindex >= 0 && _iindex < improvement_count()); \ } else { \ int _uindex = utype_index((c).value.utype); \ -assert(_uindex >= 0 && _uindex < game.control.num_unit_types); \ +assert(_uindex >= 0 && _uindex < utype_count()); \ } \ }\ } while(0); Index: ai/aitools.c === --- ai/aitools.c(revision 13913) +++ ai/aitools.c(working copy) @@ -59,9 +59,10 @@ #include "aitools.h" /** - Return a string describing a unit's AI role. + Return the (untranslated) rule name of the ai_unit_task. + You don't have to free the return pointer. **/ -const char *get_ai_role_str(enum ai_unit_task task) +const char *ai_unit_task_rule_name(const enum ai_unit_task task) { switch(task) { case AIUNIT_NONE: @@ -83,11 +84,35 @@ case AIUNIT_HUNTER: return "Hunter"; } + /* no default, ensure all types handled somehow */ assert(FALSE); return NULL; } /** + Return the (untranslated) rule name of the ai_choice. + You don't have to free the return pointer. +**/ +const char *ai_choice_rule_name(const struct ai_choice *choice) +{ + switch (choice->type) { + case CT_NONE: +return "(nothing)"; + case CT_BUILDING: +return improvement_rule_name(choice->value.building); + case CT_CIVILIAN: + case CT_ATTACKER: + case CT_DEFENDER: +return utype_rule_name(choice->value.utype); + case CT_LAST: +return "(unknown)"; + }; + /* no default, ensure all types handled somehow */ + assert(FALSE); + return NULL; +} + +/** Amortize a want modified by the shields (build_cost) we risk losing. We add the build time of the unit(s) we risk to amortize delay. The build time is claculated as the build
Re: [Freeciv-Dev] (PR#39831) Training and combat
http://bugs.freeciv.org/Ticket/Display.html?id=39831 > "Joan Creus" <[EMAIL PROTECTED]> wrote: > >http://bugs.freeciv.org/Ticket/Display.html?id=39831 > > >Good suggestion. I like it. Just one point: shouldn't it be "with Barracks" >instead of "with a Barracks"? I'm not a native English speaker, but it >doesn't feel right. > >Can anybody improve on Randy's suggestion? I suggest some wording that refers to the name of the EFT (effect) that having a Barracks or Sun Tzu causes. I can't remember the name of it at the moment (I'll just call it 'upgrade' in this email). The reason for doing so is that it is more portable - i.e. the wording will be appropriate for rulesets that may use other buildings or wonders that cause the upgrade effect. Consider this wording: "Land units sometimes gain 'veteran' status by defeating an enemy unit in combat. Also, land units can be created with 'veteran' status if they are created in a city where 'upgrade' is in effect. The 'upgrade' can be caused by buildings (e.g. Barracks), Wonders (e.g. Sun Tzu), or other factors defined in the ruleset." Replace 'upgrade' with the name of the EFT and see how it sounds. I still can't remember the name. Sorry. HTH. -Eddie ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39833) AutoReply: Citizen Manager cannot work with Gold deficit of more than 20.
http://bugs.freeciv.org/Ticket/Display.html?id=39833 > This transaction appears to have no content This patch implements the suggestion, by only testing if surplus is less than minimal surplus if minimal surplus is greater than -20. This is, literally, a 1-line change. However, all the UIs would still show -20 in the CM editing screen. Index: common/aicore/cm.c === --- common/aicore/cm.c (revision 13913) +++ common/aicore/cm.c (working copy) @@ -542,7 +542,7 @@ output_type_iterate(stat) { fitness.weighted += surplus[stat] * parameter->factor[stat]; -if (surplus[stat] < parameter->minimal_surplus[stat]) { +if (parameter->minimal_surplus[stat] > -20 && surplus[stat] < parameter->minimal_surplus[stat]) { fitness.sufficient = FALSE; } } output_type_iterate_end; signature.asc Description: PGP signature ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev