jonbaer Posted August 4, 2017 Author Report Share Posted August 4, 2017 Man just downloaded Alpha 22 and my Restart/Reseed option didn't show up - did my patch get rejected? 1 Quote Link to comment Share on other sites More sharing options...
Lion.Kanzen Posted August 4, 2017 Report Share Posted August 4, 2017 6 minutes ago, jonbaer said: Man just downloaded Alpha 22 and my Restart/Reseed option didn't show up - did my patch get rejected? Now the team is using Phabricator can be nice you add to the site, you will have more possibility to get committed. Quote Link to comment Share on other sites More sharing options...
wowgetoffyourcellphone Posted August 4, 2017 Report Share Posted August 4, 2017 Restart Match is insanely needed. 1 Quote Link to comment Share on other sites More sharing options...
Lion.Kanzen Posted August 4, 2017 Report Share Posted August 4, 2017 2 minutes ago, wowgetoffyourcellphone said: Restart Match is insanely needed. Yes three screens for saving. Quote Link to comment Share on other sites More sharing options...
wowgetoffyourcellphone Posted August 4, 2017 Report Share Posted August 4, 2017 9 minutes ago, Lion.Kanzen said: Yes three screens for saving. A simple restart option in the menu drop down would suffice at the least. Quote Link to comment Share on other sites More sharing options...
Lion.Kanzen Posted August 4, 2017 Report Share Posted August 4, 2017 1 minute ago, wowgetoffyourcellphone said: A simple restart option in the menu drop down would suffice at the least. Yes, I'm not saying the otherwise. But that patch is before they starting to do things in the Phabricator? Quote Link to comment Share on other sites More sharing options...
jonbaer Posted August 4, 2017 Author Report Share Posted August 4, 2017 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. Quote Link to comment Share on other sites More sharing options...
elexis Posted August 4, 2017 Report Share Posted August 4, 2017 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. 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. 2 Quote Link to comment Share on other sites More sharing options...
jonbaer Posted August 4, 2017 Author Report Share Posted August 4, 2017 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. Quote Link to comment Share on other sites More sharing options...
wowgetoffyourcellphone Posted August 4, 2017 Report Share Posted August 4, 2017 @elexis you guys have thrown 10x more code at solving smaller conveniences than this. Reducing 5 clicks to 2 is worth it. 1 Quote Link to comment Share on other sites More sharing options...
Lion.Kanzen Posted August 5, 2017 Report Share Posted August 5, 2017 Please elexis this small feature. Quote Link to comment Share on other sites More sharing options...
Sandarac Posted August 5, 2017 Report Share Posted August 5, 2017 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 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.