puzzlepaint / freeage

An open-source reimplementation of the game engine of Age of Empires 2: Definitive Edition.
59 stars 4 forks source link

Player stats class #9

Closed MaanooAk closed 4 years ago

MaanooAk commented 4 years ago

I implemented the PlayerStats (#6) with the basic building, unit and population info.

All the game logic boils down to these methods:

  void UnfinishedBuildingChange(BuildingType buildingType, int d);
  void FinishedBuildingChange(BuildingType buildingType, int d);
  void UnitChange(UnitType unitType, bool death, int d);
  void Change();

As the number of types are very small (and there will be only number of players instances of the struct), I stored the per type info in arrays, however in the future (if needed) it can easyly be replaced with a map.

I used the PlayerStats in the client and added the TownCenter button under the needed condition.

I understand that the server needs its own things, however I think the server could use the PlayerStats (or a class that inherits form PlayerStats) to avoid the duplication of the basic game logic that keeps track of these stats.

The only diference at the moment is the populationIncludingInProduction which is only on the server, which could be added to the PlayerStats because it's info that the client does not need (at the moment) but it's not something that is not known/calculatable to client form the rest of the info.

Awesome code base by the way

MaanooAk commented 4 years ago

I'm happy with how its now, I'm waiting on feedback and your opinion about try using the class also in the server

puzzlepaint commented 4 years ago

Thanks, from reading over it it looks good to me! I agree with using plain arrays instead of a map for now (can probably be kept this way, unless some indexing is adopted that leaves large gaps between the types).

We could merge the current state already for the client side if you want, or you could continue with the server side in this PR.

MaanooAk commented 4 years ago

Now the server also using the PlayerStats. I added a population in production counter, but for now is only used by the server, I see you changing things on the server so I would suggest to push this now.

To make easier the code, I adde a gaiStats field, for now is to just avoid checking about gai, but in the future it may used by something...

I'm waiting on feedback.

puzzlepaint commented 4 years ago

Thank you very much for the implementation! I haven't tested it yet but it looks good. Previously to this PR I was a bit concerned that some parts of the game logic code started to accumulate all over the place, and this PR introducing the PlayerStats for both the client and the server feels like a step in the right direction to improve the design again.

I will merge this now, potential minor discussion points can still be addressed afterwards.

I see you changing things on the server so I would suggest to push this now.

I only moved the pathfinding code to its own file to clean up a bit. I think it should not conflict with your changes.

Isn't the "nature" player spelled "gaia", at least in English? I haven't seen it being spelled "gai" only before. I might adjust this sometime after the merge.

MaanooAk commented 4 years ago

gai is a typo, I will fix it with my next pr... Edit: I will also fix the ci build fail