bugFreeciv - Bugs: bug #20567, Map generation retry mapseed not...

 
 
Show feedback again

bug #20567: Map generation retry mapseed not reproducible

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Wed 27 Feb 2013 07:30:30 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: None
Planned Release: 2.5.0, 2.6.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)

Tue 16 Jul 2013 06:23:02 AM UTC, SVN revision 23051:

Made games, where map generation first fails and is then retried, reproducible.

See bug #20567

(Browse SVN revision 23051)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 16 Jul 2013 06:22:56 AM UTC, SVN revision 23050:

Made games, where map generation first fails and is then retried, reproducible.

See bug #20567

(Browse SVN revision 23050)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 11 Jul 2013 08:18:50 AM UTC, comment #11:

- Start positions are not based on mapseed
- Mapseed does not depend on time() but only on gameseed

(file #18250)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Fri 19 Apr 2013 04:13:07 PM UTC, comment #10:

Should at least somehow fix the reproducibility problem in current code.

I had autogame to fail, but cannot reproduce with the 3 values I'm given: gameseed, original mapseed (failed mapgeneration), final mapseed. Gameseed does not proceed correctly if I give final mapseed to begin with.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 28 Mar 2013 12:45:11 PM UTC, comment #9:

> wanted players to study in advance


I was that close to commit my patch when it sink in that this use-case might be the reason startposition assignment has not been using mapseed. While you may want terrain to be known to everyone, at the same time you may don't want to give up where other players start.
But is thuis use-case obsolete anyway now that we have "reveal map in game start" -feature?

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Thu 28 Mar 2013 12:23:02 PM UTC, comment #8:

> if the mapseed triggered a retry.


Note that there's retry only if random mapseed is requested in the first place. No point in retrying with same set seed.
If user explicitly set mapseed, (s)he probably did it for a reason and providing map with another seed seems like a wrong thing to do.

> Is there any reason why mapseed shouldn't follow purely from
> gameseed without the time() factor?


I too think that for a given gameseed you should get same mapseed (or same mapseedS if first one triggers retry). At the same time I don't know math related to random number generator well enough to say how to make "good" mapseed with gameseed fc_rand(), so I'm not rushing with a patch.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun 24 Mar 2013 10:47:41 AM UTC, comment #7:

> But why do you think games where first map generation success
> with random seed are reproducible?

I think I must have been thinking of games with explicitly set mapseed, although I didn't make that clear.
For some reason in my autogame script I explicitly set both gameseed and mapseed for a new run.
More generally, the main case I see for only setting one seed is gameseed=0 mapseed=<chosen>, if you've got a randomly generated map you really liked, or wanted players to study in advance or whatever. That doesn't work right now if the mapseed triggered a retry.

I agree that with mapseed = 0 but explicitly set gameseed, you can't currently expect to get reproducible results regardless of retries.

That generally seems like an odd configuration to ever use, given that with a different map the game will go differently anyway. Is there any reason why mapseed shouldn't follow purely from gameseed without the time() factor?

Jacob Nevins <jtn>
Project Administrator
Sun 24 Mar 2013 06:19:24 AM UTC, comment #6:

- Game random state proceeds the same way for same map seed regradless if it the map seed was explicitly set or randomly selected

(file #17532)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun 24 Mar 2013 06:10:17 AM UTC, comment #5:

My patch has reproducibility problem in my own regression testing use-case. I use random seeds to get different games, but log those seeds carefully for using in reproduction of the problems encountered. But with my patch main random state goes one step forward (to produce mapseed) when mapseed is initially zero = for original testrun. Once one sets mapseed explicitly to same value, gameseed does not proceed.

> Which I think means any autogame where the first map generation
> failed will not be reproducible.


Uhm, I've been setting also mapseed for some time for reproducible games as I've had problems with gameseed only. And this may explain why. But why do you think games where first map generation success with random seed are reproducible?

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon 04 Mar 2013 11:48:37 PM UTC, comment #4:

> The map generation random seed would be the same on the second
> attempt as on first if main gameseed would have been succesfully
> restored after first attempt.

time(NULL) is munged into map.server.seed when it's zero in map_fractal_generate(), which I think explains this?

(Which I think means any autogame where the first map generation failed will not be reproducible. It feels to me that the right thing to do in that case is to continue subsequent map generations with the mapseed random state, i.e. move the random state management out of map_fractal_generate() entirely.)

Jacob Nevins <jtn>
Project Administrator
Mon 04 Mar 2013 11:43:03 PM UTC, comment #3:

Hm.
Not directly relevant to the patch (which seems like a fine idea), but the random Lua-generated labels, it would be kind of nice if they were generated from mapseed rather than gameseed, so that if I pick a mapseed I like, I get fixed labels as well.
But would it perhaps be too much to force every Lua function hooked off the "map_generated" signal to get mapseed-derived randomness? If we did, we'd want to ensure that game-start events could have access to gameseed-derived randomness; is the ability to hook onto the turn_started signal sufficient? I'd rather not explicitly expose two different random sources concurrently to Lua.

Jacob Nevins <jtn>
Project Administrator
Wed 27 Feb 2013 08:10:26 AM UTC, comment #2:

Turned out that start position assignment does not use mapseed, but main gameseed, so the main random state has been changing in mapgenerator. This does not explain lua script randomness problems, but attached patch anyway changes this so that mapseed is used for startposition assignment too, and the main random state is not restored to the point before initializing mapseed from it, but initialization of mapseed does push random state forward as seen by the map generator caller.

(file #17358)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed 27 Feb 2013 07:49:52 AM UTC, comment #1:

There must be some problem in how main gameseed is restored after map has been generated (note that it's already restored by the time map_generated lua signal).

Actually, if restoring of gameseed worked like it seems to be designed, our new 2.4 feature of trying to generate map with different seed if first fails would not work. The map generation random seed would be the same on the second attempt as on first if main gameseed would have been succesfully restored after first attempt.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed 27 Feb 2013 07:30:30 AM UTC, original submission:

Noticed in bug #20543. Supposedly random() values in lua script give same result in every game.

api_utilities_random() seems unnecessarily complicated. If fc_rand() works as it should, shouldn't "min + fc_rand(max + 1 - min)" be sufficient here?

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #18250:  Mapseed-3.patch added by cazfi (1kB - text/x-diff)
file #17532:  Mapseed-2.patch added by cazfi (2kB - text/x-diff)
file #17358:  Mapseed.patch added by cazfi (1kB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by cazfi (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 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 16 Jul 2013 06:23:13 AM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Thu 11 Jul 2013 08:18:50 AM UTCcazfiAttached File-=>Added Mapseed-3.patch, #18250
      StatusNone=>Ready For Test
      Planned Release=>2.5.0, 2.6.0
      SummaryLua random() not really random=>Map generation retry mapseed not reproducible
    Sun 24 Mar 2013 06:19:24 AM UTCcazfiAttached File-=>Added Mapseed-2.patch, #17532
      CategoryNone=>general
    Wed 27 Feb 2013 08:10:26 AM UTCcazfiAttached File-=>Added Mapseed.patch, #17358
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup