widelands-dev team mailing list archive

[Holger Rapp <SirVer@xxxxxx>]

> (should I send the replay to mailing list?)

yes :). Contacting one dev individually is nearly always the wrong decision. 

Also, you should make a merge request if you want to have your branch reviewed. And it should be ready for inclusion into trunk from your point of view, otherwise reviewing is a waste of time. 

> But the summary of situation is here:
> 
> My tiborb-ai branch (merged to trunk in revision 6978) contained a bug that did not show on linux only on other platforms.
> 
> You tried to fix it in revison 6989, but the fix is wrong, valgrind reports memory issues and the std::list productionsites is treated incorrectly, every time a building is to be checked (all production buildings are checked periodically), the buiding is deleted from list and this leads to absurd behaviour. AI just doesnt know about own buildings. Visible effect is that no mines are built because mines are built only if there are 8 production buildings. And AI just believes actual number of buildings is too low (decreasing by one by every regular productionsites check - done every +- 20 seconds).

Most buildings should be repushed at the end of the list before returning the function. But my changes might easily contain errors of course - I find the AI code hard to understand and there are no tests, so refactorings break things. Note that no dev has the right to complain about his/her code to be broken by others if there are no tests :).

> In tibor-ai2 I added further changes to the AI logic and hopefully fixed the crashing. Tested under valgrind, gdb - tenths of hours of gameplay and other hundereds without debug tool. But I cannot test it under other platforms.

While that sounds like a lot of testing, reality shows that just letting the game run is usually poor testing - it will not test corner cases often enough. And hundreds of hours is is just 1 hour for a hundred people - i.e. they will hit corner cases much quicker. Adding tests for your new features (AND doing the run the game for a while tests) is usually much better. 

> So options are:
> a) we go object pointers way (your design) but somebody (you?) should fix it in trunk first, I am not able to
> b) we omit object pointers and stay with old design - this is what is done in my branch.

Why not do both and use object ptrs? They are the correct choice in the sense of defense programming. I am fine with your changes as long as they do not crash the game. A bad AI is preferable to a crashing one for sure though. 

> What I suggest: somebody should test under windows/mac and find out it it still crashes and if not - I will merge trunk into my branch - option b.
> 
> And this is also reason why I did not do any merging of trunk to my branch.
> 
> Personally - I spend some time working on this, I feel it is in reasonable state and would like to have it merged finally.

Okay, then propose it for reviewing and request windows and mac feedback. You will likely not get much until it is merged and hits the nightlies though - then is when people are really starting to play with it. 


> 
> 
> Tibor
> 
> 
> 2014-06-28 8:12 GMT+02:00 Holger Rapp <SirVer@xxxxxx>:
> Please merge Trunk Now. Make it a habit to keep your branch up to date with trunk by always merging. It will make reviewing easier by making the diff as realistic and small as possible.
> 
> Also, explaining why trunk ai is broken and how you fixed it will make it easier to comment on this part directly.
> 
> > Am 27.06.2014 um 23:22 schrieb Tibor Bamhor <tiborb95@xxxxxxxxx>:
> >
> > Hi all,
> >
> > Please review latest changes I did to AI in branch tibor-ai2
> >
> > bzr branch lp:~widelands-dev/widelands/tibor-ai2
> >
> > AI in trunk is still badly broken and I think it is time to fix it.
> >
> > I am interested especially in stability on Windows and Mac platform, as this was a problem with previous iteration of AI.
> >
> > If everything is OK, I will merge trunk there and propose merging into trunk
> >
> > Thanks!
> >
> > Tibor
> > _______________________________________________
> > Mailing list: https://launchpad.net/~widelands-dev
> > Post to     : widelands-dev@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~widelands-dev
> > More help   : https://help.launchpad.net/ListHelp
>