HomeWildfire Games

Move the ability to hold a turret to a separate file.
AuditedrP23856

Description

Move the ability to hold a turret to a separate file.

The logic concerning visible garrison points (i.e. turrets) is moved from GarrisonHolder to a separate file.
This is logical because garrisoned != turreted, so this allows for turrets that cannot be garrisoned (refs. D1958).
Also references #3488.

Differential Revision: D2367
Reviewed by: @wraitii

Event Timeline

Nescio raised a concern with this commit.Jul 20 2020, 12:59 PM
Nescio added a subscriber: Nescio.
Nescio added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

<GarrisonHolder> → <TurretHolder> messes up alphabetical order of template nodes.
Moreover, I don't really understand why it's necessary.

<VisibleGarrisonPoints> → <TurretPoints> makes sense.

How about the following?

<GarrisonHolder>
  <TurretPoints>
    <One>
      <X>0</X>
      <Y>9.3</Y>
      <Z>0</Z>
    </One>
  </TurretPoints>
</GarrisonHolder>
This commit now has outstanding concerns.Jul 20 2020, 12:59 PM
Freagarach added inline comments.Jul 20 2020, 1:27 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Good point about the ordering ^^' D2896

The reason it has become a separate component is because there is much logic different in how to handle a garrisoned entity vs a turreted entity and them being together became quite awkward.

Nescio added inline comments.Jul 20 2020, 1:31 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

So is it now possible to have an entity with <TurretPoints> but not a <GarrisonHolder> node?

Freagarach added inline comments.Jul 20 2020, 1:37 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Yes, but we don't have the possibility to occupy that point yet ^^
I'm going to work on the turrets patch first (that needs this). Then we can look further into the split.

Nescio added inline comments.Jul 20 2020, 1:38 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Good to know.
Also, why <TurretHolder> and not simply <Turrets>?

Freagarach added inline comments.Jul 20 2020, 1:43 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Because the component defines the ability to hold turrets not only the turret points. It may well be that in the future it has more template data than just the turret points and renaming the component then is (also for modders) more work (it can obviously be scripted) than just adding a template value.

Nescio added inline comments.Jul 20 2020, 1:48 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Yes, I understand the node levels, I just meant the name:

<Turrets>
  <TurretPoints>
    <One>
      <X>0</X>
      <Y>9.3</Y>
      <Z>0</Z>
    </One>
  </TurretPoints>
</Turrets>
Nescio added inline comments.Jul 20 2020, 1:51 PM
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Anyway, it's not important, I was just curious.

Freagarach marked 4 inline comments as done.Jul 20 2020, 1:55 PM
Freagarach added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/templates/structures/athen_wall_long.xml
6–8

Yeah, I understand what you meant :) That was what I tried to explain. To me, turrets looks like it is only a list of (possible) turret points, although it can define more than just that.

Anyway, it's not important, I was just curious.

Yeah, fine :)

Itms awarded a token.Jul 20 2020, 2:31 PM

It seems this causes units on gates to interfere with closing them again:

Stan added a subscriber: Stan.Jul 20 2020, 3:46 PM

cf D2775 maybe needs some extra code in Gate.js

Might be in UnitAI -> "Immobile" is probably not set in the new codepath... Not obvious why at a glance though.

Silier raised a concern with this commit.Jul 20 2020, 4:11 PM
Silier added a subscriber: Silier.
Silier added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
283

!! missing

Freagarach marked 2 inline comments as done.Jul 20 2020, 5:00 PM
Freagarach added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
283
Nescio removed an auditor: Nescio.Jul 21 2020, 10:58 AM
Silier accepted this commit.Jul 21 2020, 8:24 PM
All concerns with this commit have now been addressed.Jul 21 2020, 8:24 PM