[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Mon Feb 25 03:32:06 2008]: Madeline Book wrote: Unfortunately for you, posting a comment about a side-effect of your patch does not imply that the person posting the comment tested your patch. But never fear, there is PR#40113. Unfortunately for all of us, posting a comment about a side-effect of a patch *DOES* imply the person has tested the patch! The policy of posting patches here is not just for the keyboard exercise Your bundled gamerule change was the only thing I was replying to as I have made clear. It is a pity that you continue to assume that I looked at your code in any detail, but if it gives you such enjoyment to continue to make this claim, who am I to correct you? ;) Moreover, PR#40113 is not reproducible on my machine(s). That's a shame. Maybe you could try a little harder? Maybe run a server one of your machines, then run a client on another machine, and have the client connect to the server and try to start a game? It is fairly complicated, but I think a freeciv user should be able to accomplish it. Hopefully, you will find the problem. I'm not sure if I am good enough! :( But I supposed I will try, for the sake of people running freeciv in a multiplayer context. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: I mean a server setting (in server/settings.c) like savepalace or killcitizen, I don't see why this should be for things (?) that change only from ruleset to ruleset Neither should be server settings. That's the old way. Most of these have been gradually moved to the (newer) effects rulesets. Likewise diplomats/spies, as that would completely destroy their stealth. Spies and diplomats do not have the Partial_Invis flag. Definitely a ruleset bug. There's some oddity with the V_INVIS layer and various hidden test functions. Probably later additions without sufficient documentation. for stealth fighters/bombers, consider that submarines can in fact be detected by their effect on city workers when their controlling player is foolish enough to move them into the field of the enemy city Again, definitely a bug. Consider also that an air or civilian unit can be used to bypass ZOC, implying that they do in fact control the tile, and hence should bounce city workers. Interesting argument. ZOC is changed considerably in civ3. Bad design decisions shouldn't be enshrined in perpetuity. However, none of this is relevant to this ticket, which is on the path to fixing crashing bugs ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Sun Feb 24 12:51:14 2008]: Madeline Book wrote: I mean a server setting (in server/settings.c) like savepalace or killcitizen, I don't see why this should be for things (?) that change only from ruleset to ruleset Neither should be server settings. That's the old way. Most of these have been gradually moved to the (newer) effects rulesets. Ah, thank you for that clarification. It does make much more sense that gameplay rules be controlled by rulesets rather than server settings (which has unfortunately been the standard practice for warserver up until now). Likewise diplomats/spies, as that would completely destroy their stealth. Spies and diplomats do not have the Partial_Invis flag. Definitely a ruleset bug. There's some oddity with the V_INVIS layer and various hidden test functions. Probably later additions without sufficient documentation. I would call it a ruleset omission rather than a bug. As far is I know diplomats and spies never had the Partial_Invis flag in the default ruleset, and players are highly cognizant of that fact. for stealth fighters/bombers, consider that submarines can in fact be detected by their effect on city workers when their controlling player is foolish enough to move them into the field of the enemy city Again, definitely a bug. I think it could be argued that this behaviour is a feature rather than a bug, but I grant that if the only way to fix crashes is to sacrifice it for the new behaviour, then it is alright to lose it. Consider also that an air or civilian unit can be used to bypass ZOC, implying that they do in fact control the tile, and hence should bounce city workers. Interesting argument. ZOC is changed considerably in civ3. Bad design decisions shouldn't be enshrined in perpetuity. I'm not sure if this last sentence refers to ZOC handling or the other arguments in the discussion. Since if it refers to ZOC it is a non sequitor, I will assume it is refering to the latter. ;) I would hope that civ3 emulation be controlled by rulesets (analogously to the civ1 and civ2 rulesets), and the core functionality of the civserver be flexible enough to satisfy the expectations of all users (multiplayer and singleplayer). However, none of this is relevant to this ticket, which is on the path to fixing crashing bugs I agree, but you did bundle a gameplay rule change into your bugfix, to which this discussion is relevant. Try to separate the two in the future. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Mon Feb 25 01:11:20 2008]: Since Madeline's testing revealed no bugs in the code execution, I never tested the code (indeed looking back at my posts I never said so implicitly or explicitly), I merely commented on your own report of the one game rule change, and why it would not be, according to me, a good idea. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Mon Feb 25 03:15:06 2008]: Madeline Book wrote: I never tested the code WTF! The whole purpose of posting patches for comments is that the patches are actively tested! Unfortunately for you, posting a comment about a side-effect of your patch does not imply that the person posting the comment tested your patch. But never fear, there is PR#40113. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: Unfortunately for you, posting a comment about a side-effect of your patch does not imply that the person posting the comment tested your patch. But never fear, there is PR#40113. Unfortunately for all of us, posting a comment about a side-effect of a patch *DOES* imply the person has tested the patch! The policy of posting patches here is not just for the keyboard exercise Moreover, PR#40113 is not reproducible on my machine(s). Hopefully, you will find the problem. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110
The first and most obvious part of the patch updates the tile/vision
documentation somewhat, as it was woefully out of date!
Renamed some of the enum known_type to reflect their source, and replaced
the order dependent = = tests.
Moved city_can_work_tile() out of server/citytools into common/city, to be
used in both client and server. This meant changing it somewhat, as it
used some server-only functions.
The upside is a new feature: air and civilian units no longer prevent
working a tile. This better conforms with usual expectations. See new
unit_occupies_tile().
Although the purpose of this series of patches is removal of conflicts
between city_map[] and tile_worked(), there was (at least) one use that
wasn't duplicated:
* seen tiles worked by cities that are not seen by the client!
The city_map[] has them marked TILE_UNAVAILABLE. In the server, the tile
is set, but not in the client.
Now, the city id is passed to the client. So the client has to make some
invisible cities (unknown center tile) to set the worked field.
That makes 2 client uses of NULL == city-tile (the other being the Editor).
The server still expects the tile field is always set.
Also, in borderless games, there's no tile_owner() to use for these
invisible cities, so an out-of-band (but in range) owner is used instead:
MAX_NUM_PLAYERS (in the reserved for barbarian range). The owner is
corrected as soon as the city is seen, or a border expands.
This required some other small modifications, primarily LOG_DEBUG.
Extensively tested on savegames of multiple bug reports. Seems to do what
is expected. I'm sure many more edge cases will be found
Index: doc/HACKING
===
--- doc/HACKING (revision 14420)
+++ doc/HACKING (working copy)
@@ -592,28 +592,29 @@
Unknown tiles and Fog of War
=
-In the tile struct there is a field
+In common/tile.h, there are several fields:
struct tile {
...
- unsigned int known;
+ bv_player tile_known, tile_seen[V_COUNT];
...
};
-On the server the known fields is considered to be a bitvector, one
-bit for each player, 0==tile unknown, 1==tile known.
-On the client this field contains one of the following 3 values:
+While tile_get_known() returns:
+/* network, order dependent */
enum known_type {
- TILE_UNKNOWN, TILE_KNOWN_FOGGED, TILE_KNOWN
+ TILE_UNKNOWN = 0,
+ TILE_KNOWN_UNSEEN = 1,
+ TILE_KNOWN_SEEN = 2,
};
-The values TILE_UNKNOWN, TILE_KNOWN are straightforward. TILE_FOGGED
-is a tile of which the user knows the terrain (inclusive cities, roads,
-etc...).
+The values TILE_UNKNOWN, TILE_KNOWN_SEEN are straightforward.
+TILE_KNOWN_UNSEEN is a tile of which the user knows the terrain,
+but not recent cities, roads, etc.
-TILE_UNKNOWN tiles are (or should be) never sent to the client. In the past
-UNKNOWN tiles that were adjacent to FOGGED or KNOWN ones were sent to make
+TILE_UNKNOWN tiles never are (nor should be) sent to the client. In the
+past, UNKNOWN tiles that were adjacent to UNSEEN or SEEN were sent to make
the drawing process easier, but this has now been removed. This means
exploring new land may sometimes change the appearance of existing land (but
this is not fundamentally different from what might happen when you
@@ -623,9 +624,10 @@
Fog of war is the fact that even when you have seen a tile once you are
not sent updates unless it is inside the sight range of one of your units
or cities.
+
We keep track of fog of war by counting the number of units and cities
[and nifty future things like radar outposts] of each client that can
-see the tile. This requires a number per player, per tile, so each tile
+see the tile. This requires a number per player, per tile, so each player_tile
has a short[]. Every time a unit/city/miscellaneous can observe a tile
1 is added to its player's number at the tile, and when it can't observe
any more (killed/moved/pillaged) 1 is subtracted. In addition to the
Index: server/cityhand.c
===
--- server/cityhand.c (revision 14420)
+++ server/cityhand.c (working copy)
@@ -185,11 +185,11 @@
return;
}
- if (!city_can_work_tile(pcity, ptile)) {
+ if (0 == city_specialists(pcity)) {
return;
}
- if (0 == city_specialists(pcity)) {
+ if (!city_can_work_tile(pcity, ptile)) {
return;
}
Index: server/citytools.c
===
--- server/citytools.c (revision 14420)
+++ server/citytools.c (working copy)
@@ -1979,40 +1979,6 @@
}
/**
- Returns TRUE when a tile is available to be worked, or the city itself is
- currently working the tile (and can continue).
- city_x, city_y is in city map coords.
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Sat Feb 23 18:08:32 2008]: The upside is a new feature: air and civilian units no longer prevent working a tile. This better conforms with usual expectations. See new unit_occupies_tile(). Please take care not to change gameplay rules without leaving some method of accessing the old behaviour (i.e. a server setting). Players expect this and are generally very annoyed when it changes for no apparent reason (I take a lot of flak even for inadvertently changing shortcuts for warclient). For example with this new behaviour it is not possible to lay seige to a city with fighters (a fairly common strategy, starving it in terms of shields and/or food). And as for appealing to realism (I assume that is what you mean by usual expectations, if not then disregard the following ;)), one could argue just as well that air units scare away the peasantry/farmers (i.e. the city tile workers), and enemy civilian units steal the tile's resources surreptitiously. The rationalization is arbi- trary; the effect on gameplay should generally the only metric by which to judge feature changes and additions. (Cf. my laborious efforts on the longturn forums arguing against using realism justifications for gameplay changes). ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: And as for appealing to realism (I assume that is what you mean by usual expectations, I meant the actual behavior of civ1/2/3. We only need options for things that change from ruleset to ruleset. Does an air unit bounce a city worker in civ1? Does an air unit bounce a city worker in civ2? Does an air unit bounce a city worker in civ3? My memory is that they do not, but it's been a long time. Likewise diplomats/spies, as that would completely destroy their stealth. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Sun Feb 24 02:57:33 2008]: Madeline Book wrote: And as for appealing to realism (I assume that is what you mean by usual expectations, I meant the actual behavior of civ1/2/3. We only need options for things that change from ruleset to ruleset. I don't understand this justification (maybe we have confused each other?). I mean a server setting (in server/settings.c) like savepalace or killcitizen, that would control the effect air and/or civilian units would have on enemy city workers. I don't see why this should be for things (?) that change only from ruleset to ruleset (is this a policy? And can you give me some examples so I better know what you mean?). Does an air unit bounce a city worker in civ1? Does an air unit bounce a city worker in civ2? Does an air unit bounce a city worker in civ3? I don't know the answer to those questions. I also think it is not relevant, unless you view strict emulation of civ1,2,3 more highly than respecting your current user base. Would not a server setting also give you the benefit of the doubt? Perhaps one version of civ has that behaviour, and another does not. I am objecting to a change in a game rule that I know will cause at least minor discomfort to players used to the behaviour before the change. Hence my suggestion that a server setting be added to allow access to the old game rule. If your structural changes to the codebase have made implementing such a setting impossible, well then alright, that's really a shame, and I hope you will be more careful about such things in the future (to avoid irrevocably changing a more important game rule). My memory is that they do not, but it's been a long time. Likewise diplomats/spies, as that would completely destroy their stealth. Spies and diplomats do not have the Partial_Invis flag. As for stealth fighters/bombers, consider that submarines can in fact be detected by their effect on city workers when their controlling player is foolish enough to move them into the field of the enemy city (a common, relied-upon occurence in games). So there is precedent for stealthy units having thier stealth destroyed by affecting enemy city workers. Consider also that an air or civilian unit can be used to bypass ZOC, implying that they do in fact control the tile, and hence should bounce city workers. I hope my advice above, representing the input of the multiplayer freeciv community, has been helpful. If you find that it is tiresome to have me second-guessing you all the time, just say so, or feel free to ignore me. ;) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
