Opened 3 years ago

Closed 3 years ago

#6081 closed enhancement (fixed)

Clean Garrison-related code.

Reported by: Freagarach Owned by:
Priority: Should Have Milestone: Alpha 25
Component: Simulation Keywords:
Cc: Patch:

Description (last modified by Freagarach)

There has been a significant effort in cleaning garrisoning-related code, yet more needs to be done.

As discussed on IRC today, the ticket is created to not forget when the person currently working on it disappears suddenly.

A list of possible things:

  • Ungarrison using cmpGarrisonable instead on cmpGarrisonHolder (refs. Phab:D3280). Phab:D3628
  • Don't listen to OnGlobalEntityRenamed messages when the only use case is Skirmish Replacements (one can use a new message for those). Phab:D3627

Change History (18)

comment:1 by Freagarach, 3 years ago

Patch: Phab:D3627,Phab:D3628

comment:2 by Freagarach, 3 years ago

Keywords: simple removed
Patch: Phab:D3627,Phab:D3628Phab:D3627 Phab:D3628

comment:3 by Freagarach, 3 years ago

Again discussed today on IRC.

  • UnitAI shouldn't probably care about garrisoning.
  • Garrisonable sets UnitAI IDLE.
  • Orders/States in UnitAI just check for being able to move.

comment:4 by Freagarach, 3 years ago

In 25019:

Ungarrison entities using cmpGarrisonable.

Follow-up to r24977 / rP24977.
This moves responsibilities even further to the garrisoning entity.

Fixes the garrison flag when renaming entities cannot garrison (#5906).
Allows easy fixing of #6087.

Ticket: #6081
Differential revision: D3628
Comments by: @Stan, @wraitii (also in length on IRC)

comment:5 by Freagarach, 3 years ago

Patch: Phab:D3627 Phab:D3628Phab:D3627 Phab:D3628 Phab:D3648
Priority: Nice to HaveShould Have

comment:6 by Freagarach, 3 years ago

Description: modified (diff)

comment:7 by Freagarach, 3 years ago

In 25029:

Use seperate message to handle skirmish replacements.

Meaning GarrisonHolders won't listen for GlobalEntityRenamed messages uselessly for the rest of the match after init.

Refs. #6081
Differential revision: D3627

comment:8 by Freagarach, 3 years ago

Description: modified (diff)

comment:9 by Freagarach, 3 years ago

In 25030:

Give the Garrisonable component the power to set garrisoned on an entity with UnitAI, instead of leaving it to UnitAI.

Since the only way of garrisoning is using cmpGarrisonable.
r9498 / rP9498 introduced the this.isGarrisoned-flag, its meaning was discussed in Phab:D1403.

Refs. #5979, #6081
Differential revision: D2379
Comments by: @elexis, @Stan, @wraitii

comment:10 by Freagarach, 3 years ago

In 25031:

Handle ownership changes of a garrisoned entity in cmpGarrisonable.

Makes sure GarrisonHolders don't need to needlessly listen to global ownership changes anymore.
Also fixes dying entities on turret points.

Refs. #6081
Differential revision: D3648

comment:11 by Freagarach, 3 years ago

In 25034:
Fix ownership changing assertion with a garrison holder with multiple garrisoned entities.

In lieu of a better solution.

Reverts: r25031 / rP25031

comment:12 by Freagarach, 3 years ago

In 25044:

Move movement logic from UnitAIs "Garrisoned"-state to approaching.

Reduces switching of states, improves readability.

Refs. #6081
Differential revision: D2368
Comments by: @Stan, @wraitii

comment:13 by Freagarach, 3 years ago

In 25062:

Handle aura and production pause on GarrisonedStateChanged message.

This reduces hard-coupling between the components.

Refs. #6081
Differential revision: D3683
Reviewed by: @wraitii

comment:14 by Freagarach, 3 years ago

In 25069:

Let garrisoned entities be IDLE.

The "GARRISONED"-state was quite strange, for entities being garrisoned can just as well perform other tasks (see e.g. turrets).
Also, the need for keeping a "garrison" order on the stack is removed.

Fixes: #6022
Differential revision: D3656
Refs. #6081

comment:15 by Freagarach, 3 years ago

In r25123:

Technically seperate Turrets from GarrisonHolder.

While they often look alike, their behaviour is totally different. This split has some implications:

  • There are now separate auras for garrisoning and turrets.
  • Entities can now have both turret points and garrison slots, independent of eachother.

In general previous behaviour is maintained as much as possible.

Differential revision: D3150
Comments by: @Nescio, @wraitii
Tested by: @v32itas

comment:16 by Freagarach, 3 years ago

Patch: Phab:D3627 Phab:D3628 Phab:D3648

comment:17 by Freagarach, 3 years ago

In 25189:

Use interval instead of timeout for healing in GarrisonHolder.

Prevents constant recreation of a timeout.

Differential revision: D3791
Refs. #6081

comment:18 by Freagarach, 3 years ago

Resolution: fixed
Status: newclosed

Seems clean enough for now. Feel free to reopen if appropriate.

Note: See TracTickets for help on using tickets.