Jump to content

Adding a "restart" option (for random maps)


jonbaer
 Share

Recommended Posts

I think this is it ... https://trac.wildfiregames.com/ticket/4284 ... what I was more interested in at the time besides a Restart option was also a Reseed ability to regenerate a random map (in case I didn't like the layout that was generated).  I may have missed it since I didn't understand what the last comment on the ticket was referring to.

Link to comment
Share on other sites

 

4 hours ago, jonbaer said:

Man just downloaded Alpha 22 and my Restart/Reseed option didn't show up - did my patch get rejected?

If it was rejected, it'd clearly state that.

That what LionKanzen said is correct. The ones doing reviews first look at the patches they are interested in, then the patches they are personally asked to be reviewed, then the open ones on Phabricator, then (almost never) the ones on trac with rfc or review keyword, then (basically never) the ones without keywords on trac.

We started Alpha 22 development with about 80 reviewable patches iirc. The handful of active developers we have spent their time doing somewhere around 400 reviews that ended in commits, maybe 100 reviews not ending in commits and barely any own coding. Now we have 65 patches on that work-in-progress milestone on trac, 205 open patches on phabricator and the will to spend much less  time on reviews. :shifty: Dunno how this should go on.

2 hours ago, jonbaer said:

I think this is it ... https://trac.wildfiregames.com/ticket/4284 ... what I was more interested in at the time besides a Restart option was also a Reseed ability to regenerate a random map (in case I didn't like the layout that was generated).  I may have missed it since I didn't understand what the last comment on the ticket was referring to.

Other than that, a quick review:

  • Perhaps hiding the Restart button would be nicer for multiplayer.
  • "New map generation" and "Same map generation" would be more clear to players than "Reseed".
  • The Seed property is also used for the random number generator in general, for example the spawned hero in regicide, so skirmish maps will spawn the same one. The Seed setting was changed to use randIntInclusive or so.
  • The AI seed could also be regenerated.
  • Replay saving should be checked (iirc EndGame and StartGame take care of that already though)
  • playerID variable not needed / can be inlined.
  • Don't remove the resign comment.
4 hours ago, wowgetoffyourcellphone said:

Restart Match is insanely needed.

Dunno if it is. Menu -> Exit -> Single Player -> Start Match -> Start. 5 clicks does the same as these proposed 2 clicks. If the player wants to change any other setting besides the seed, he will still have to go through that route. We don't have a way to start the same map with the same Seed, so that'd be something new.

Also I recall FeXoR saying that we could offer a Seed setting in the match setup. But that might also lead to confusion (as players dont necessarily know what a seed is) and that changing the teams, civs or other match settings makes the Seed setting irrelevant (misleading that it ends up with the same mapgen) and also would require a way to automatically reset the seed unless really wanted by the player. Dunno. Could be committed I guess if it's so desired.

  • Like 2
Link to comment
Share on other sites

I'd actually like to put this onto a GitHub pull request along w/ another patch (Random AI Settings), would that be easier?  The thing is I don't see any PR(s) on the current master that is up there.  I had to delete my SVN copy for needed space for work related stuff - I'd like to see the entire project move to GItHub but I do recall there being issues w/ the larger files.  (Seeing a split between engine code + public mod would be great too but I know how difficult that would be to keep in sync).

My other option was to just create mod(s) for both against Alpha22 + just post them to GitHub repo.  Either way great job handling this project in an open source setting - I'd imagine it is not an easy task.

Link to comment
Share on other sites

8 hours ago, jonbaer said:

I'd actually like to put this onto a GitHub pull request along w/ another patch (Random AI Settings), would that be easier?

No, as others have already alluded to, if you want your patch to be properly reviewed and possibly committed, it should be ported to Phabricator (notwithstanding the glut of unreviewed patches). Github pull requests are not used in the development process.

See: https://trac.wildfiregames.com/wiki/SubmittingPatches and https://trac.wildfiregames.com/wiki/Phabricator

  • Like 1
Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...