patchFreeciv - Patches: patch #3908, Map size rounding for...

 
 
Show feedback again

patch #3908: Map size rounding for tilesperplayer map size selection.

Submitted by:  Micke <mss_8734>
Submitted on:  Mon 13 May 2013 08:36:33 PM UTC  
 
Category: generalPriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Mon 13 May 2013 09:50:01 PM UTC, comment #1:

I'm not convinced this part of the code is the issue. I can generate a 20% discrepancy anyway (tilesperplayer=60, landmass=30, mapsize player, 5 players). Everything is evenly divisible, so it falls into the second part of the conditional, but I get a map of 1200 tiles (of which typically more than 350 are land, and I've gotten numbers as high as 389, without enabling tinyisles). At a high level, this is a 20% overage (1200 vs 1000), and much of that is available at a lower level. This doesn't seem to be running against the minimum size (512 tiles), so I blame something in set_sizes(). The same 5 players typically get an underage (4800 from 5000 is only 4%) with tilesperplayer=300.

Not to say that this isn't also a potential issue, but since we're counting in thousands for map.server.size, increasing a value of 1.5 from 1 to 2 only reduces us from being 33% wrong to being 25% wrong. We could probably be more accurate if we had more significant digits (and wouldn't need rounding), so we'd be explicitly requesting 1500 tiles for 20% landmass, 5 players, 60 tiles per player, rather than trying to decide which of 1000 or 2000 is a closer match. Given the requirements of map cardinality and defined ratios, I'd argue that sizing/fitting issues cause greater discrepancy than rounding if we carried all the digits.

On a side note, if this code is being improved, it may be worth bounding it so that size can't be 0: this causes the server to crash being unable to construct a map (e.g. landmass 45%, 5 players, 60 tiles per player): this rounding will make it harder to hit this condition, but it may still be possible with some input conditions.

Emmet Hikory <persia>
Project Member
Mon 13 May 2013 08:36:33 PM UTC, original submission:

Whilst playing around with the map generator code I noticed that the resulting map often has less than 85% of the requested landmass when using tilesperplayer. Applying this patch yields a smaller discrepancy by some crude rounding.

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -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):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 13 May 2013 08:36:33 PM UTCmss_8734Attached File-=>Added mapsize_tpp_rounding.diff, #17961
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup