bugFreeciv - Bugs: bug #20710, Editor can't place Maglev in alien...

 
 
Show feedback again

bug #20710: Editor can't place Maglev in alien or experimental rulesets

Submitted by:  Emmet Hikory <persia>
Submitted on:  Thu 04 Apr 2013 07:58:13 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: 2.4.99-dev-r22663Operating System: None
Planned Release: 2.5.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 08 Apr 2013 10:16:55 PM UTC, SVN revision 22696:

Add road dependency roads recursively when adding road in editor.
It used to add just immediate dependencies of the target road.

Reported by Emmet Hikory

See bug #20710

(Browse SVN revision 22696)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun 07 Apr 2013 08:32:45 AM UTC, comment #5:

Your patch is ideal for the editor aspect. Autosettlers is more complicated, so better deferred to another bug or issue.

Emmet Hikory <persia>
Project Member
Sun 07 Apr 2013 08:07:03 AM UTC, comment #4:

We then agree that my patch is sufficient for S2_5 at least? I have some plans to make threaded AI generally to consider "more than one step" of improvements. Some groundwork for that has already been made, but I think we will see actual benefits only during 2.6 development. The idea here is that threaded AI is allowed to be more computationally expensive than classic ai as long as it happens midturn, not while human players are waiting ai to finish turn change activities.

As for avoidance of recursive code: We have nothing against it as long as you know what you're doing, and avoid very deep recursion. Here I check against infinite recursion (road A depending on B depending on A) and max depth is max number of road types. We've had problems with infinite recursion, and something like going through entire map by making recursive call for tiles neighbour tiles.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 06 Apr 2013 05:56:09 PM UTC, comment #3:

Thinking about this in more depth, I'm less certain it works quite as well as I thought. While informing autosettlers about maglev needing road seems to make them make more roads, I think they do it for the wrong reasons: they are looking at maglev+road, rather maglev+highway/railroad+road to determine the time and benefits. To do what I thought it should do would require using a recursive iterator to set the total dep_time and dep_value for the full stack of roads, which gets expensive (and probably repetitious). For now, ruleset authors with multistep roads should ensure that either the next road to be built or the one after that has a tile bonus or expect that the road will only be built for transportation purposes.

As much as I think there remains an autosettler bug (causing autosettlers to prefer roads less than they should), that's a sufficiently different issue that it's not worth conflating them (for all that testing my initial naive solution happened to appear to work).

Emmet Hikory <persia>
Project Member
Fri 05 Apr 2013 02:12:38 AM UTC, comment #2:

Heh, and I was trying to avoid recursive code (based on all the comments in the existing code discussing recursion avoidance). Your patch is a much better solution.

For autosettlers, the issue is the perceived value of building a road: if they have not been informed that building a road is required to build a maglev, then they will not consider the benefits of building a maglev as potential future benefits of building a road. Depending on the precise effects associated with maglev (or any road with recursive requirements), this may not sufficiently elevate the value of building the basic road high enough for autosettlers to consider it important.

The relevant logic is in the road_deps_iterate loop in autosettlers.c:settler_evaluate_improvements(). Changing this to also use the recursive loop would increase the perceived benefit of roads lower in the dependency stack, causing autosettlers to build roads earlier/more often in the event that top layers of the stack provide useful benefits.

Emmet Hikory <persia>
Project Member
Thu 04 Apr 2013 09:41:28 PM UTC, comment #1:

I rather have this typical case of roads being upgrades to each other handled by the code.

Attached patch for the editor part. I'm not so concerned abpout autosettlers not building Roads in order to build Maglevs - they should build roads anyway and then they can upgrade them all the way to Maglevs. I haven't checked the code, but I'm not sure your patch even worked in that respect - maybe autosettlers notice that the tile cannot currently have Maglev (due to missing Railroad/Highway) even if it had Road.

(file #17678)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 04 Apr 2013 07:58:13 AM UTC, original submission:

The implementation of road_deps_iterate requires that the full set of potential requirements for a road be listed in the reqs{} section, or that road cannot be placed in the editor on unimproved terrain and autosettlers will not consider the base dependency road as a target if there is a desire for the road with recursing requirements.

For 2.6+, this should probably be addressed by some richer generalised requirements_tree_walk feature, but for 2.5 the simple fix is to update the rulesets to include recursively required roads, for which a patch is attached.

For a test condition, try placing a Maglev on unimproved terrain with either the alien or experimental rulesets before and after this ruleset change.

Emmet Hikory <persia>
Project Member

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #17678:  RecursiveRoads.patch added by cazfi (2kB - text/x-diff)
file #17674:  recursive-road-reqs.patch added by persia (1kB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by cazfi (Updated the item)
  • -unavailable- added by persia (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 08 Apr 2013 10:17:08 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Thu 04 Apr 2013 09:41:28 PM UTCcazfiAttached File-=>Added RecursiveRoads.patch, #17678
      Categoryrulesets=>general
      StatusNone=>Ready For Test
      Planned Release=>2.5.0
    Thu 04 Apr 2013 07:58:13 AM UTCpersiaAttached File-=>Added recursive-road-reqs.patch, #17674
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup