[Freeciv-Dev] [patch #3826] Allow bases on city tiles
Update of patch #3826 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Update of patch #3826 (project freeciv): Status:None = Ready For Test Assigned to:None = cazfi Planned Release: = 2.5.0, 2.6.0 ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #14, patch #3826 (project freeciv): Shipping rulesets have no bases surviving in the cities, but if we are adding it as a feature for ruleset, it should work. Note that almost any ruleset where some base can exist in city is subject to superior nation conquered city from weaker enemy -case. We've been trying to get major releases out more frequently, but fact is that there usually is years between them - 2.6 release (hopefully with extras -framework) is far away (we've not released 2.4! yet) ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #15, patch #3826 (project freeciv): My apologies. I suppose I'm used to significantly less mature projects, where a feature freeze tends to happen only immediately prior to release (but releases correspondingly tend to be buggier without the overlap). Attached is a patch with the trivalue logic. I noticed while producing this that we unconditionally remove obsolete buildings on city takeover: while I think it outside the scope of this patch (improvements don't currently use negated requirements to indicate obsolescence, nor do roads/bases have a cache of potential obsolecense conditions), it probably makes sense to review this code section for 2.6, and give the same treatment for improvements and extras: the new player may not want some obsolete infrastructure which no longer provides a useful bonus (or may even cause a penalty, depending on applicable effects). (file #17815) ___ Additional Item Attachment: File name: allow-bases-in-cities+less-stunning.patch Size:37 KB ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #9, patch #3826 (project freeciv): There's no other callers via fc_funcs-destroy_base(), but there's direct callers in server grep -r destroy_base server/ ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #10, patch #3826 (project freeciv): Oh, sure. These all assume that destroy_base() will call tile_remove_base(). I was mostly concerned about callbacks, because they could potentially *not* call tile_remove_base(), which would require the extra call in tile_change_terrain(). I'll open a different patch to drop the redundant call, and leave a comment indicating that tile_change_terrain() relies on any callback set for fc_funcs-destroy_base() to perform tile_remove_base(). ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #11, patch #3826 (project freeciv): Redundant call of tile_remove_base() now patch #3876 ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #12, patch #3826 (project freeciv): As you are handling road and base upgrades separately, I assume some messages to player be suboptimal if there's both road and base upgradet at the same time. Namely: Will it say The people ... stunned by your technological insight! twice when city is conquered, or New hope sweeps like... if new tech allows both road and base type? ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #13, patch #3826 (project freeciv): Do you think it's worth adding trivalue logic for 2.5, when it will all be swept away for 2.6 with extras? At least for the shipping rulesets, there are no cases of this. ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #5, patch #3826 (project freeciv): Then I noticed how roads and bases are handled differently when new city is founded. Existing roads remain, but bases are removed unless city owner would be able to build new such base. I think you should be removing only those bases (and roads, for consistency) that cannot exist in city, regardless if player is able to build them. This might even already happen (for both or roads only) outside this code. ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #6, patch #3826 (project freeciv): Updated patch adjusts base destruction to not be player-specific (so that players may build cities on city-compatible bases they could not build themselves, and keep the base), as well as removing any city-incompatible roads when founding a city. During the production of this patch, I noticed what looks like a duplicate call to tile_remove_base() in tile.c:tile_change_terrain(): is this a bug, or is there some reason to double-remove the base here because of the nature of fc_funcs that I don't understand? (note that destroy_base() calls tile_remove_base() itself if used.) (file #17773) ___ Additional Item Attachment: File name: allow-bases-in-cities+road-removal.patch Size:36 KB ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #7, patch #3826 (project freeciv): I don't think there's any actual need for double removal, but otherwise destroy_base() would need to either be passed boolean parameter telling if caller is removing base itself, or it would need to be able to deduct the need itself. Some callers other than tile_change_terrain() need it. Or maybe tile_change_terrain() could remove base only if callback is not set, and to rely on *any* callback ever assigned there to take care of the removal. ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #4, patch #3826 (project freeciv): The attached patch should address all comments, and applies against trunk. Thanks for pointing out how roads/bases are removed when terrain is changed. For some reason, in the initial testing, I wasn't successful in restricting bases in cities from the ruleset without passing the city, but it seems to work now, and I cannot remember the specific failing test. (file #17762) ___ Additional Item Attachment: File name: allow-bases-in-cities+comment-fixes.patch Size:36 KB ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #3, patch #3826 (project freeciv): - git commit message claims that pcity is needed for requirements preventing building base/road in cities, but CityTile, Center, Local requirement is used for that. It's probably a good idea to pass that city information to requirement parsing, but it's out of scope for this patch. - FIXME comment about removal of roads/bases from city tiles should be removed, or changed to point out that in that sense city tiles act like any other tile - extras are removed in tile_change_terrain() ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
URL: http://gna.org/patch/?3826 Summary: Allow bases on city tiles Project: Freeciv Submitted by: persia Submitted on: Tue 02 Apr 2013 04:29:57 PM GMT Category: general Priority: 5 - Normal Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: ___ Details: This patch allows one to build bases in cities, allows cities to have inherent bases (for example, if one wanted some basic infrastructure that was best implemented with a base and upgraded appropriately with the tech tree), and allows one to only have *some* bases or roads automatically built for cities. All of this only supports ruleset authors: there should be no change in the behaviour for any shipping rulesets. Note that I'm a little uncertain about whether to set AlwaysOnCityCenter or AutoOnCityCenter flag for road_type road in civ1, civ2, and civ2civ3 rulesets, as I don't have enough historical experience playing them, nor have I ever played the games they apparently intend to model. I've done my best based on guessing from various comments in the code or reference websites, but if someone knows the intended behaviour better, these values could be adjusted. For alien, it doesn't matter because there is no requirement for Bridge Building. For classic/experimental/multiplayer, historical behaviour has been to provide a road even on river tiles prior to the discovery of Bridge Building, so AlwaysOnCityCenter is correct unless someone had intentionally changed this behaviour. Also note that while I've set Buoys to be automatically destroyed if a city is built upon them, in most of the rulesets, this is irrelevant, as they don't permit Oceanic cities: I added this mostly so that anyone performing minimal modifications to add oceanic cities wouldn't suddenly end up with buoys not being deleted. Lastly, I'm not sure I understand precisely why the previous code wanted to block base effects before creating the virtual city, but also couldn't find a good way to eliminate unsuitable bases much earlier in create_city(): there may be some interaction between the effects of unsuitable bases and some of the earlier actions taken while setting up the city. I don't believe the call can be any earlier than tile_set_owner() and map_claim_ownership(), because at that point pcity doesn't get defined in player_can_build_base(). ___ File Attachments: --- Date: Tue 02 Apr 2013 04:29:57 PM GMT Name: allow-bases-in-cities.patch Size: 37kB By: persia http://gna.org/patch/download.php?file_id=17657 ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #1, patch #3826 (project freeciv): whether to set AlwaysOnCityCenter or AutoOnCityCenter flag for road_type road in civ1, civ2, and civ2civ3 civ1/civ2 always had the road on city center. As up to freeciv-2.4, freeciv always gave, and expected, road to all city center tiles, including oceanic ones! AlwaysOnCityCenter is correct unless someone had intentionally changed this behaviour. That was intentionally changed when it became possible to have city center without road (one of the first commits to TRUNK after S2_4 was branched). Idea was to test how it works in gameplay and somehow change it if need arises. So far nobody has complained. Note that code has evolveld so that when city center *needs* road was first changed to roads requirements are always respected, it was impossible to have ruleset with road always on city center. AlwaysOnCityCenter flag was only later introduced. ___ Reply to this item at: http://gna.org/patch/?3826 ___ 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 #3826] Allow bases on city tiles
Follow-up Comment #2, patch #3826 (project freeciv): Aha! revision 21864. Sorry about that. allow-bases-in-cities+fixed-rulesets.patch should restore the prior behaviour properly, unless I missed something between 21864 and 22649 that further adjusted the values. (file #17658) ___ Additional Item Attachment: File name: allow-bases-in-cities+fixed-rulesets.patch Size:37 KB ___ Reply to this item at: http://gna.org/patch/?3826 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
