[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-30 Thread Marko Lindqvist
Update of patch #3826 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at:

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-28 Thread Marko Lindqvist
Update of patch #3826 (project freeciv): Status:None = Ready For Test Assigned to:None = cazfi Planned Release: = 2.5.0, 2.6.0

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-23 Thread Marko Lindqvist
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-23 Thread Emmet Hikory
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-22 Thread Marko Lindqvist
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-22 Thread Emmet Hikory
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().

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-22 Thread Emmet Hikory
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!

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-22 Thread Marko Lindqvist
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-22 Thread Emmet Hikory
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-19 Thread Marko Lindqvist
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-19 Thread Emmet Hikory
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-19 Thread Marko Lindqvist
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-19 Thread Emmet Hikory
Follow-up Comment #8, patch #3826 (project freeciv): Hrm? maphand.c:destroy_base() unconditionally calls tile_remove_base() as the last operation. On the server, fc_funcs-destroy_base is set to maphand.c:destroy_base(), and on the client, fc_funcs-destroy_base is set to NULL. I don't see any

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-18 Thread Emmet Hikory
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-11 Thread Marko Lindqvist
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,

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-02 Thread Emmet Hikory
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

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-02 Thread Marko Lindqvist
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,

[Freeciv-Dev] [patch #3826] Allow bases on city tiles

2013-04-02 Thread Emmet Hikory
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)