jsettlers / settlers-remake

A Remake of "The Settlers III" for Windows, Linux, Mac and Android
http://www.settlers-android-clone.com
MIT License
354 stars 100 forks source link

Implement winning #771

Closed tehKaiN closed 4 years ago

tehKaiN commented 5 years ago

fixes #689

Stuff that works:

Things I'm unsure implementation-wise

Things I'm unsure of codestyle-wise:

tehKaiN commented 4 years ago

Quoting @paulwedeck's message on discord:

Why arent you calling disableFogOfWar directly in WinLoseTracker#timerEvent ? As far as I know Actions are there to notify any potential listeners of an UI interaction So atleast the Action stuff should be removed Maybe add something like getVisibleStatusPlayer in MainGrid.GraphicsGrid that returns the IPlayer from FogOfWars constructor If this is the just determined victor you call mainGrid.disableFogOfWar()

So I rewrote my code so that it disables FoW from WinLoseTracker's timerEvent()

andreas-eberle commented 4 years ago

@tehKaiN: Ternary and early returns are both allowed. In general, we aim to write readable code ;)

andreas-eberle commented 4 years ago

@tehKaiN: Thanks for your PR! Keep up the good quality!

I just found three small issues:

I fixed these issues and will merge it when CI is green.

tehKaiN commented 4 years ago

Thanks for bug finding! I'll try to test my next changes more thoroughly.