Fix map sharing

[lmoureaux]

Sharing maps between players was fundamentally broken. The server records the last turn in which a player saw a tile. This information is only updated when the player stops seeing a tile, and not when it just keeps seeing it because the tile is within its vision range.

The following map sharing bugs existed:

• Tiles seen by the giving player were not sent if the receiving player had last viewed them after the giving player;
• Cities were only updated when destroyed. Renamed or growing cities were not updated.

This commit fixes both bugs. The core logic in really_give_tile_info_from_player_to_player was completely overhauled to make it easier to follow and more correct. In addition, player_tile.site was turned to a unique_ptr to facilitate bookkeeping.

Closes #2045

Testing: Before the patch:

• Watch the occupied city indicator in a city. Move units in and out while giving your map to another player. See the indicator not being updated for the other player.
• Same with city renaming.
• If you are patient enough, repeat with city growth, terrain alterations, etc.

After the patch, the other player should always get the information at the time of signing the treaty.

Not suggesting to backport since the bugs had been there for a very long time.

[jeltsch]

Tiles seen by the giving player were not sent if the receiving player had last viewed them after the giving player

Is this really a bug? I would expect that newer information doesn’t get overwritten with older information.

[lmoureaux]

Tiles seen by the giving player were not sent if the receiving player had last viewed them after the giving player

Is this really a bug? I would expect that newer information doesn’t get overwritten with older information.

"Seen" tiles are the tiles currently visible by a player (as opposed to tiles that are merely "known")

[jwrober]

Looks like the PR is failing the clang-format checker.

[lmoureaux]

Uh I see, the usual case not caught by git-clang-format

[lmoureaux]

clang-format fixed

[jeltsch]

Tiles seen by the giving player were not sent if the receiving player had last viewed them after the giving player

Is this really a bug? I would expect that newer information doesn’t get overwritten with older information.

"Seen" tiles are the tiles currently visible by a player (as opposed to tiles that are merely "known")

Then I don’t understand this even more. If the seen tiles are currently visible to the player, they are viewed now. How can then the other player have them last viewed at a later time?

[lmoureaux]

The two players are usually not sharing vision, as this is related to the treaty clauses to share one's map

[lmoureaux]

Sorry, I missed the question in the second part. As an optimisation (I suppose), the "last viewed" info is not updated at every turn change. It gets updated only when vision is lost.

[jeltsch]

Sorry, I missed the question in the second part. As an optimisation (I suppose), the "last viewed" info is not updated at every turn change. It gets updated only when vision is lost.

I must say that finally I’m utterly confused. I don’t know whether I’d like to help here further. This stuff seems so convoluted that I don’t really have further ambition to try to understand it. Sadly, I’m by no means convinced that the fix in this PR is actually suitable.

[jeltsch]

Looking again at what “last viewed” means, it starts to make sense to me why the first point mentioned in the PR description could be a bug. Still, parts of the map where the receiving player has currently vision should not be sent. Is this taken into account by the updated code?

[lmoureaux]

parts of the map where the receiving player has currently vision should not be sent. Is this taken into account by the updated code?

Yes, this is the first check:

  // No need to transfer if pdest can see the tile
  if (map_is_known_and_seen(ptile, pdest, V_MAIN)) {
    return;
  }

I'd be interested if you have ideas for a simpler implementation

[jeltsch]

I'd be interested if you have ideas for a simpler implementation

At least not spontaneously. :slightly_smiling_face:

[lmoureaux]

@jwrober can you review then?

[jwrober]

@jwrober can you review then?

Yep. Prolly tomorrow. Will need to setup a test.

[lmoureaux]

Thanks!

[jwrober]

Works for me. Nice patch!