Garrison

[MaanooAk]

Almost full support for the garrison mechanics. For the full list check the commit names, I think all features have a separate commit (I don't think there is a need to list here again).

Left to do in the future (except from using the type stats instead of hard-coded values):

• Mechanics
  • Ungarrison a unit from a unit to empty space around (much easier to implement when there will be a unit with garrison capacity)
  • Town center bell (+ villager ungarrison and go back to work)
• Gui
  • Add health bars to the garrison units icons
• Game logic related
  • If relic, keep track of number of relics in PlayerStats
  • Bonus attack to some buildings from garrisoned units
  • Bonus speed to some units from garrisoned units
  • Heal garrisoned units

(Update list or copy it to an issue or to a todo list ?)

[puzzlepaint]

For the network message of the garrison action, probably a variant of the SetTarget message could be suitable. For example, one could make a new Garrison message type and reuse CreateSetTargetMessage() for creating it, adding a parameter that specifies the interaction type (default, garrison, repair).

[MaanooAk]

@puzzlepaint I have been using a new SetTargetWithInteraction (placeholder name), however there some things that I had to change in the InteractionType model thing, I will make a commit later today, we can discuss over the code.

[MaanooAk]

I am stopping adding things, so you can review the code. I think its a good point to stop here, almost all the garrison basic features are here. I updated the pr description with what is left to do (I think).

[MaanooAk]

I created the ungarrison message (please take a look at the packing and unpacking, I'm not very comfortable writing this kind of code...).

I tried to do what you said about getting rid of the bool, this is what I refactored it to:

  bool GarrisonUnit(u32 unitId, ServerUnit* unit, u32 targetObjectId, ServerObject* targetObject);
  bool UngarrisonUnit(u32 unitId, ServerUnit* unit, u32 targetObjectId, ServerObject* targetObject);
  void UngarrisonAllUnits(u32 targetObjectId, ServerObject* targetObject);

  /// Change the garrison state of a unit and the targets object garrisoned units list. 
  /// The enter parameter is true for garrison and false for ungarrison.
  /// Returns true if the unit's garrison status changed.
  bool ChangeUnitGarrisonStatus(u32 unitId, ServerUnit* unit, u32 targetObjectId, ServerObject* targetObject, bool enter);

I think now the server side is much cleaner, I'm sorry about the previous mess (I had just forgot about it..)

[puzzlepaint]

Thank you very much for the update! I'll merge this then. Just to emphasize this, if you don't want to do some suggested changes, then that would be totally fine as well. Personally, when I contributed to other projects, I didn't particularly like having to re-visit PRs that I thought to be done, so I would fully understand that ... I would put the suggestions on my to-do list then and implement them myself once I get to it.

The message packing / unpacking looks fine to me. I think the main point is to properly check the message lengths and validate everything that is received via the network to prevent possible attacks from malicious clients. (As you probably noticed, the way the indexing into the buffers is being done is a mess, sometimes the 3-byte header is included and sometimes it is not, this could surely be simplified.)

[MaanooAk]

@puzzlepaint Nice to know, but if I see a suggestion for improvement of my code that I'm capable of implementing, I can't just leave it...