patchFreeciv - Patches: patch #3906, Road movement cost granularity.

 
 
Show feedback again

patch #3906: Road movement cost granularity.

Submitted by:  Micke <mss_8734>
Submitted on:  Mon 13 May 2013 12:56:03 PM UTC  
 
Category: generalPriority: 5 - Normal
Status: DuplicatePrivacy: Public
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Planned Release: 

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)

Thu 20 Mar 2014 11:54:11 AM UTC, comment #6:

Looking at the implementation in patch #3990, it's much cleaner than the patches I submitted. It also seems to support the multiplication by 10 in the original submission. Can this ticket now be closed, or is there something else outstanding?

Emmet Hikory <persia>
Project Member
Thu 11 Jul 2013 06:06:40 AM UTC, comment #5:

My approach in patch #3990 is to make number of fragments ruleset configurable.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 13 May 2013 05:36:08 PM UTC, comment #4:

I forgot to mention, it's important to do the MOVE_COST_IGTER patch first, as otherwise one has to somehow calculate MOVE_COST_IGTER in relation to SINGLE_MOVE, and hope that it just happens to work for a given value.

Emmet Hikory <persia>
Project Member
Mon 13 May 2013 05:09:44 PM UTC, comment #3:

I'm mostly working on requirement processing right now, but here are a couple untested candidate patches that implement items 1) and 2). If someone would like to test/adjust/fix these to actually work properly, they can have all the credit (large chunks are mechanical and if nothing else need adjustment to meet the line length guidelines of CodingStyle. I believe they belong in separate tickets. These are currently stacked over all sorts of patches in the review queue, so may not apply to trunk, but should at least point out the sorts of changes that would be needed.

Although you claim not to have the skill to write the patches, you've demonstrated the ability to read code, think about code, and submit patches: I suspect you would be able to adapt/debug/review the untested patches. Note that I may have missed something in these patches: they were mostly generated by searching for uses of the constants and making them go away: adjusting rulesets to use the changed code and playtesting is absolutely required as a bare minimum before they could be considered for application.

For road flags, I'm waiting for deeper review/application of patch #3832 or lack of other things to do to add the user-defined ones, but wouldn't mind at all if someone else chose to write that before I get to it :). Allowing these to affect the movement mid-move would require much more complex handling in tile_move_cost_ptrs(), and extends beyond my current thinking.

(file #17958, file #17959)

Emmet Hikory <persia>
Project Member
Mon 13 May 2013 04:53:13 PM UTC, comment #2:

I agree completely, but fear I lack the skill to implement this properly. Perhaps the movement cost for roads could be changed to a percentile as well, instead of a fixed number of movement points. Maybe even a flag for roads to toggle overriding/adding to terrain movement cost?
User defined road flags (or accepting the terrain flags) would be great too, but I suppose that might warrant a separate ticket.

Micke <mss_8734>
Mon 13 May 2013 03:00:43 PM UTC, comment #1:

I really like the idea of this patch, but I think there is a lot more that could be done to make this flexibly work, as follows:

1) Move SINGLE_MOVE to be a ruleset configuration variable (e.g. game.info.single_move), so that ruleset authors could select the degree to which they wanted to be able to adjust granularity (for example, with the suggested patch, there is no means to make a road cost 1/4 of SINGLE_MOVE). If the default in ruleset.c is set to 3, this has the added advantage that no adjustments are required for current rulesets (although one may wish to add comments to make the current rulesets a better source for new ruleset authors). For the ruleset modification envisioned for this patch, the ruleset would simply set this value to 30.

2) Change MOVE_COST_IGTER to be an optional unit type parameter, so that different unit types could have different maximum travel costs: this allows one to specify some units that can travel as easily over mountains as hills (utype->move_cost_igter = SINGLE_MOVE*2), but still move faster on plains, or units that move like there is a road everywhere even in the case where roads have a move_cost of 2: if stacked over patch #3900, this ought be fairly easy to implement (pass int igter_move_cost rather than bool igter to tile_move_cost_ptrs(), with '0' meaning !UTYF_IGTER, add igter_move_cost to pf_parameter, and set it when filling parameters in pf_tools.c, and pass it in single_move_cost()). This should default to 1 to reduce ruleset transition difficulties. Extra points for finding a good name for a unit class configuration variable to define the default scaling between movement points and actions (e.g. attacking) for !UCF_TERRAIN_SPEED classes (such nomenclature being one of the reasons I haven't submitted a patch to do this yet).

Personally, I had planned to submit these as separate tickets, but they are both relevant to the content of this patch.

A third change that helps ruleset authors adjust granularity would be to replace movement_cost for terrains with move_cost, so that one could have terrains that default to less than a single move (e.g. hardpan saltflats which oughtn't be harder to navigate than roads) without needing to prepopulate the map with lots of invisible roads on the target terrain (and then deal with the road coordination movement issues about source and destination discussed in patch #3829).

Emmet Hikory <persia>
Project Member
Mon 13 May 2013 12:56:03 PM UTC, original submission:

Attachment 1 multiplies the movement cost factors defined in common/movement.h by 10, which enables greater road granularity.
Attachment 2 adapts all rulesets that come with a default installation (data/*/terrain.ruleset).
Both are made with trunk rev. 22860.

Micke <mss_8734>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #17958:  0001-Untested-max_move_cost-patch.patch added by persia (25kB - 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 (Posted a comment)
  • -unavailable- added by persia (Posted a comment)
  • -unavailable- added by mss_8734 (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 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 20 Mar 2014 05:52:30 PM UTCcazfiStatusNone=>Duplicate
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Mon 13 May 2013 05:09:44 PM UTCpersiaAttached File-=>Added 0001-Untested-max_move_cost-patch.patch, #17958
      Attached File-=>Added 0002-Untested-SINGLE_MOVE-replacement-patch.patch, #17959
    Mon 13 May 2013 12:56:03 PM UTCmss_8734Attached File-=>Added move_roads_granularity.diff, #17956
      Attached File-=>Added move_roads_granularity_rulesets.diff, #17957
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup