[Freeciv-Dev] [bug #13843] [Patch] Build base menu
Follow-up Comment #8, bug #13843 (project freeciv): can_units_do_base() and can_units_do_base_gui() return essentially the same information, but with two different code paths and different tests. You are welcome to improve the situation if it's possible, but... - Both of them use can_build_base() deep down, so tests are the same - can_units_do_base() use can_unit_do_activity_base() which is same test server uses to determine if request send by client is valid. This way menu item sensitivity is guaranteed to match what server will accept. This is also identical to how most of the other sensitivity menu items are handled. - There is nothing matching can_units_do_base_gui() in server side. Client itself decides what actual base should be built, and sends normal base building request. - can_unit_do_activity_base() does not - and cannot - know anything about base gui_type ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Update of bug #13843 (project freeciv): Status: In Progress = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #7, bug #13843 (project freeciv): OK, that makes sense - much better to fix the problem deeper down. Shows how much I have to learn about the depths of the code. :-) Still, I can't help but wonder: can_units_do_base() and can_units_do_base_gui() return essentially the same information, but with two different code paths and different tests. Besides applying your patch, wouldn't it be good to also normalize these two functions? (Not necessarily the way I did it, but something to prevent discordances like this, should the base functions change again.) ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #5, bug #13843 (project freeciv): Sorry, Marko. I suppose I should have given more info so that you could reproduce. Steps: * with a unit capable of it, build a fortress (either with new Build Base Fortress or with old Build Fortress) * after fortress is built, now look at available commands for the same unit on that tile ** Fortress under Build Base is insensitive ** Build Fortress is still sensitive The above also applies to Airbase. The above steps should show the inconsistency: 1) The new commands reflect whether the base type can be built by the unit as well as whether there is already a base of that type on the current tile. 2) The old commands only reflect whether the unit has the technical ability to build, not whether there is already a base of that type present on the current tile. My patch changed the check for the old commands so that their sensitivity was the same as for the new commands under the Build Base menu. ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #4, bug #13843 (project freeciv): Please post new patches to new tickets. I almost forgot your patch as it's in already closed ticket! It looks like can_units_do_base() checks to see if units can build a base type and can do that activity on the current tile, whereas can_units_do_base_gui() only checks to see if units can build a base type. I cannot reproduce nor do I see that problem in source code of can_units_do_base_gui(). It calls get_base_by_gui_type() with punit-tile as parameter to check which base, if any, is buildable there. ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #3, bug #13843 (project freeciv): Thanks for taking the time to explain more in-depth, Marko! That really helps me to understand the context. I've make changes to my (upcoming) menu reorg patch. I have some better ideas, but for now I punted too and left the menu as-is. :-) On the other hand, your new code reveals an inconsistency in how the existing Build Fortress and Build Airbase have their sensitivity set. It looks like can_units_do_base() checks to see if units can build a base type *and* can do that activity on the current tile, whereas can_units_do_base_gui() only checks to see if units can build a base type. I've attached a patch that makes can_units_do_base_gui() behave the same as can_units_do_base(). (file #6170) ___ Additional Item Attachment: File name: unitlist.diff Size:0 KB ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #2, bug #13843 (project freeciv):
This new submenu lists all buildable base types - ruleset can specify
arbitrary number of them. This is quite similar to Change Government
submenu.
Having almost duplicate entries for fortresses and airbases is far from
optimal. Nobody has yet come out with good suggestion how to handle this in a
better way. We cannot reserve keys for all possible base types, so items in
Build Base menu have none. Inreased moddability should not make use of
default ruleset harder, so Fortress and Airbase in default ruleset should
keep their keys ('f' 'e'). Old entries under Orders menu refer to these.
Also, clients other than gtk have no generic base building possibility
similar to Build Base menu.
Entries under Build Base build exactly the selected base type. Build
Fortress and Build Airbase on the other hand build first available base
type that has gui-type Fortress or Airbase, respectively. First
available may depend on terrain, unit type, known technologies... So it's
even possible that if you have several focus units and select Build
Fortress, some of the focus units start building Fortress and others
Tower.
Hope this helps to understand current situation.
___
Reply to this item at:
http://gna.org/bugs/?13843
___
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] [bug #13843] [Patch] Build base menu
Update of bug #13843 (project freeciv): Status:None = Fixed Assigned to:None = cazfi Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
Follow-up Comment #1, bug #13843 (project freeciv): I'm sorry I'm posting after this bug is closed, but it's of interest to me. I'm in the middle of redoing a menu layout change that I originally submitted about a year ago. Would you be able to go into a bit more depth about why this path moves items into a submenu? I don't quite understand the context (I'm way behind on list reading, so please point me to the appropriate thread if there was a discussion). Also, these seem to make the two old entries redundant, but they remain in the orders menu (Build Airbase and Fortify/Build Fortress). ___ Reply to this item at: http://gna.org/bugs/?13843 ___ 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] [bug #13843] [Patch] Build base menu
URL: http://gna.org/bugs/?13843 Summary: [Patch] Build base menu Project: Freeciv Submitted by: cazfi Submitted on: Wednesday 07/01/2009 at 20:11 Category: client-gtk-2.0 Severity: 3 - Normal Priority: 1 - Later Status: None Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Release: Operating System: None ___ Details: This patch adds Build Base submenu with all buildable base types under Orders menu of gtk client. This far only one base with gui type Fortress and one with gui type Airbase has been selectable for building at any one time. This limitation is not as bad as it first appears. This set of two base types can be different for different units, terrain types, technological level... ___ File Attachments: --- Date: Wednesday 07/01/2009 at 20:11 Name: BaseMenu.diff Size: 4kB By: cazfi http://gna.org/bugs/download.php?file_id=6103 ___ Reply to this item at: http://gna.org/bugs/?13843 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
