[PATCH] Implement: Allies share vision an unlocking technology

[michael]

I would like to make Allied shared vision an unlocking technology ("Cartography" or something along these lines).

Basically, allies start the match without shared vision, but then unlock shared vision after researching a technology. This is a standard thing in Age of Empires games.

[leper]

Do both players have to research this for it to work (I think so but it's been some time since I last played AoE)

[historic_bruno]

Replying to leper:

Do both players have to research this for it to work (I think so but it's been some time since I last played AoE)

Not in AoK, as soon as you build a market and research cartography, you get LOS of your allies. I'm not sure if I like this more as a technology, or something you get automatically at a certain phase, or as a diplomatic option, or some combination.

[Raymond]

I disagree.

[Adrián Chaves]

Note: even if we disagree to implement this for 0 A.D. — Empires Ascendants, it might be useful to be able to have this, modding-wise.

[Niek]

Implements the Shared LOS technology

[Niek]

Patch with style fixes

[sanderd17]

The actual tech is missing from your second patch, but it's in your first, so it doesn't hinder a review (would be nice to get them together in your next patch though).

I have a bit of a bad feeling using ApplyValueModifications for this. The method was meant for numeric template-defined values. Besides, it should always operate on the base value, not on the updated value.

I think it would be better to do something similar to template requirements: Define in the player template how the shared los technology is called, then, when a valuemodification happens, check if that tech is researched, and use that immediately as flag to set the shared los or not.

[Niek]

Different approach

[sanderd17]

Pretty well done. The only issue I have is, when there's no shared los tech defined for the player, I would assume it keeps the old behaviour (so share los immediately), instead of the current early return.

[leper]

Not sharing LoS seems like a nice default IMO.

[Niek]

Fix a style issue

[mimo]

carthography -> cartography

[mimo]

In fact, thinking again to this patch, I'm a bit bothered by the fact that we have to give the name of the tech in the player.xml file and test on this name for applying the tech. That looks a bit as a hack for me

Maybe adding a new propriety in the techs to say that it should apply to the player entity

"playerModifications": [{"value": "Player/SharedLos", "replace": true}],

And add in the player.xml, something like <Player>

<SharedLos>false</SharedLos>

</Player>

would be better, and more flexible for modding.

[Niek]

Fix typo

[Niek]

Move return up in function and add comment to schema

[sanderd17]

In 16795:

Allies only share vision when researching a tech. Based on patch by niektb. Fixes #2055

[Itms]

In 16842:

Correctly compute the masks for shared visibility updates. The old code was working only because of luck when shared vision was always reciprocal.

Refs #2055, fixes #3327