HomeWildfire Games

Technically seperate Turrets from GarrisonHolder.

Description

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

Event Timeline

What does <Turretable/> do? Allow units to be placed on turret slots? Shouldn't crossbowmen have it too then? Or perhaps all infantry (see outpost)?
It used to be possible to specify what classes could occupy a specific slot, e.g. allowing a structure to have one slot for slingers and four others for archers. Is that still true?
And does this mean units on visible slots no longer contribute to recovering capture points? If so, it's a gameplay change.

Turretable indeed allows an entity to be placed in a turret point. One can still specify the entities that may occupy specific slots.
(Regarding the crossbowmen, I forgot to rebase this after that.)

Good call on the recovering of CP. That is indeed changed now -_-' To me it seems for the better (if you fight you can't retake CP).

Why is <Turretable/> necessary? And shouldn't all units have it (cf. <Garrisonable>)?

One can still specify the entities that may occupy specific slots.

Good to know! I actually think it would be better to explicitly set it to Ranged Infantry on each wall slot, and Infantry on the outpost.

Because the Turretable component contains the logic for occupying (and leaving) a turret, e.g. setting obstruction and turret stance in UnitAI. Not all entities need it, no only those who were able to turret before. I know infantry was allowed on an outpost, so that is indeed changed now. Might or might not be desired. IIRC people were requesting changes to the outpost anyway.

You never know what mods want to do, though:

But mods can just add the component to template_unit?

True. My point is visible slots are something specific to the entities (typically structures) that have them and the proper way would be to define what enities (typically units) can go into those slots in the files where the slots are defined, as used to be the case (with the outpost template specifying Infantry and the wall parent template Ranged+Cavalry). This commit effectively defines it in an arbitrary selection of units.

Not really an arbitrary selection, but I get your point.
It is still possible, just not done ('cause it was way more work and programmers are lazy).

And another thing: I can no longer choose and click to eject individual units in the left selection panel:

Nope, that left panel was only there for hidden entities. I advocated to leave the button at the bottom out as well, but people disagreed.

It does make walls a bit harder to use, though.
And what if an entity has both (internal) GarrisonHolder and (external) TurretHolder?

Per @wraitii's request goes the GarrisonHolder first.

(When there is a different icon, we can change the hotkeys to be seperate (they're the same now).)

Yes, that would certainly help.

Can't you temporarily use the pack.png icon for now? It may not be perfect, but having a single button for two functionally different things is a rather problematic

Nope, that left panel was only there for hidden entities. I advocated to leave the button at the bottom out as well, but people disagreed.

Hm, I actually meant that the left-panel should still show turrets at the moment, too. I definitely agree with Nescio on that.
(When we have a better interface for turrets, we'll see what needs be done).

Stan added a subscriber: Stan.EditedMar 26 2021, 4:38 PM

Here is a mockup idea. Drag & drop would be cool, to switch turret point

Nice, @Stan! :) How would that look with e.g. twice as many turret points (e.g. the Mauryan tower).

Stan added a comment.Mar 27 2021, 2:15 PM

Maybe like this I think 12 might be good enough for most people.

Langbart raised a concern with this commit.Mar 28 2021, 12:02 AM
Langbart added a subscriber: Langbart.
This comment was removed by Langbart.
This commit now has outstanding concerns.Mar 28 2021, 12:02 AM
Langbart resigned from this commit.Mar 28 2021, 12:03 AM
This commit no longer requires audit.Mar 28 2021, 12:03 AM

@Stan, it certainly looks nice! However, the left selection panel has a width of 5 icons. Medium wall segments have 4 slots, long wall pieces 8, gates up to 10 (the siege wall gate could have 16 if the lower platform level were used), and the unused double tower of the Mauryas 16. And one never knows how many slots might want to add to their structures (42?).
Moreover, what if multiple entities with slots are selected simultaneously (e.g. double-click on a wall piece)?

I think reusing the garrison panel with a small icon indicating 'turret point' would be fine enough for now TBH.

Yeah, I agree. See how it's done in the right selection panel: if you select both units and structures, then the panel simultaneously displays (un)locking gates, (un)packing artillery, trainable units, and buildable structures:


Furthermore, as @Freagarach mentioned here earlier, a separate button and hotkey is needed for emptying slots.

Silier added a subscriber: Silier.May 1 2021, 2:43 PM
Silier added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/Garrisonable.js
153

takes only 1 parameter

@Freagarach GarrisonManager and AIProxy?

Silier raised a concern with this commit.Nov 28 2021, 8:02 AM
This commit now has outstanding concerns.Nov 28 2021, 8:02 AM

@Freagarach GarrisonManager and AIProxy?

Not affected?

How no?
Ai gets information through proxy and messages.
If she doesn't get memo that unit is in turret state, she will act like it would not be.

But she wouldn't use that information at all?

Why to give option to occupy turret then to api?

Completeness. ^^ But adding it to the Proxy will affect performance.

Its not complete now.
If someone would want to use it, they would need to update proxy and other stuff in common api.

Also performance is not the question since number of messages for turreted is just subset of original number of messages from garrisoned.
Only additional cost are extra variables but amount of data is not affected.

Stan added a comment.Jul 6 2022, 10:59 AM

I think it's fixed now that you no longer need the proxy for the AI?

Stan removed an auditor: Silier.Aug 8 2022, 9:08 AM
This commit no longer requires audit.Aug 8 2022, 9:08 AM