rtfmcorp / asylamba-game

Jeu de stratégie spatial en ligne multi-joueur.
http://asylamba.com
22 stars 19 forks source link

Fix issue 147 #152

Closed tinydetoon closed 7 years ago

tinydetoon commented 7 years ago

Change OrbitalBaseRepository Update method to include Building levels updates in the query.

Kern046 commented 7 years ago

Hi there !

I just saw your pull request, thanks for the work !

The building fields cannot be updated in this method. I removed it due to some heavy bugs (the subprocesses often committed changes to the dataset in the same time than other scripts, so the final building level wasn't right). Instead, a new method must be implemented, a dedicated method to the base change type. You can implement it in the repository and then call it in the changeBaseType script instead of the unit of work flush method !

Don't hesitate to contact me if you have further questions :)

tinydetoon commented 7 years ago

Hello Kern,

I have updated accordingly to your comments. Hope it is better now

Kern046 commented 7 years ago

It's all good to me ! Thanks for your work :D !

acknowledge commented 7 years ago

@tinydetoon Can you just remove the useless spaces you added in some places (see here). And for your new method, please use the correct indentations (tabs instead of spaces).

A tiny commit and I'll merge ;)

tinydetoon commented 7 years ago

I have clean the code. It should be better now. I will pay more attention for the next commits ;-)

acknowledge commented 7 years ago

Thanks for cleaning the extra spaces.

But still, can you indent you method updateBuildingLevels with tabs instead of spaces. So that all the code uses the same conventions ;)

tinydetoon commented 7 years ago

Should be good !

acknowledge commented 7 years ago

Ok, here we are. Please analyze this commit. It contains what I asked you to do. You indented your code with 4 spaces, but have to indent with 1 tabulation. And the indentation should be correct.

Now here some advices for you next contributions:

Ok that's it. Thanks for your contribution. And I hope my advice will help you in the future.

tinydetoon commented 7 years ago

Thanks for your advices and sorry for the rework you had to do. It seems to be a configuration in my netbeans.

acknowledge commented 7 years ago

No problem ;) Yes probably, on most editors (or IDEs), you can tune this kind of parameters.