r12035+: make stop public on shared halt also changes other players' tiles

[Nazalassa]

Title says all. With the patch, when a player makes a shared halt public, only their tiles turn public. Ownership is updated accordingly.

Note: I was not sure what counts towards a player having ownership in a halt: having buildings/platforms, or just tracks, or wayobjs? The patch assumes only buildings/platforms count. If otherwise, set "tiles_converted" to "true" in haltestelle_t::change_owner() whenever needed.

[victor_18993]

Hi,

I had a look at this. The cause is in haltestelle_t::change_owner(). That function was written back when a stop had a single owner, so it converts every tile of the halt to the public player regardless of who actually owns it. On a shared halt that also turns a co-owner's buildings and ways public, which is wrong: only the tiles of the player who uses "make public" should change (or all of them when the public player itself is the one doing it, same as before).

There is a second, related problem right after the loop. The owner mask was set with "owners = 1 << player->get_player_nr()", which throws away any remaining co-owner of the halt. I rebuild it from the actual tile owners instead, the same way add_grund() already does it.

What the patch does:

• change_owner() now also receives the initiating player (old_owner). Buildings and ways are only converted when they belong to old_owner, or unconditionally when the public player is the initiator so the existing behaviour is kept. The way-only branch of the tool already used exactly this check, so this just makes change_owner() consistent with the code next to it.
• the owner mask is recomputed from the tiles after the conversion.
• the single caller in tool_make_stop_public_t::work() passes the initiating player.

I also added a regression test (test_halt_make_public_shared): it builds a public halt shared by the public player and two companies, makes it public from one company and checks that the other company's tile is left untouched. The whole test suite passes, and the new test fails without the fix.

Patch attached (unified diff, applies with patch -p1 / git apply on a recent trunk).

Regards,
victor_18993

Versión en español

Hola,

Le he echado un vistazo a esto. La causa está en haltestelle_t::change_owner(). Esa función se escribió cuando una parada tenía un único dueño, así que convierte todas las tiles del halt al jugador público sin importar de quién son realmente. En un halt compartido eso también vuelve públicos los edificios y vías de un co-dueño, lo cual es incorrecto: solo deberían cambiar las tiles del jugador que usa "hacer pública" (o todas, cuando es el propio jugador público quien lo hace, igual que antes).

Hay un segundo problema relacionado, justo después del bucle. La máscara de dueños se fijaba con "owners = 1 << player->get_player_nr()", lo que descarta a cualquier co-dueño que quede en el halt. En su lugar la reconstruyo a partir de los dueños reales de las tiles, igual que ya hace add_grund().

Lo que hace el parche:

• change_owner() ahora recibe también el jugador que inicia la acción (old_owner). Los edificios y vías solo se convierten cuando pertenecen a old_owner, o incondicionalmente cuando el iniciador es el jugador público, de modo que se conserva el comportamiento existente. La rama de la herramienta que hace pública una vía suelta ya usaba exactamente esta comprobación, así que esto solo hace que change_owner() sea coherente con el código de al lado.
• la máscara de dueños se recalcula a partir de las tiles tras la conversión.
• el único llamador en tool_make_stop_public_t::work() pasa el jugador iniciador.

También añadí un test de regresión (test_halt_make_public_shared): construye un halt público compartido por el jugador público y dos empresas, lo hace público desde una de las empresas y comprueba que la tile de la otra empresa queda intacta. La suite de tests completa pasa, y el nuevo test falla sin el arreglo.

Parche adjunto (diff unificado, se aplica con patch -p1 / git apply sobre un trunk reciente).

Un saludo,
victor_18993

[prissi]

Added with some more fix on owner and error message because this tool should also allow transfer between players.