Update of bug #23444 (project freeciv):

        Operating System:               GNU/Linux => Any                    
         Planned Release:                   2.5.1 => 2.5.1, 2.6.0, 3.0.0    

    _______________________________________________________

Follow-up Comment #6:

So what I think is happening is that the client's 'transporter' (cargo) and
'transporting' (transport) state is getting out of sync. The 'transporting'
unit list of the transport is not being populated with the cargo.

This is caused by the server sending the client the cargo before the transport
on initial attach / game load, and the client failing to catch up when
knowledge of the transport eventually arrives.

On the server, there's code that attempts to send transports before cargo, but
it's buggy:


  unit_transports_iterate(punit, ptrans) {        
    fc_assert_action(i < ARRAY_SIZE(info), break);
    package_unit(punit, &info[i++]);              
  } unit_transports_iterate_end;                  


package_unit(punit, ...) should be package_unit(ptrans, ...)
I've tested that fixes this fixes the symptom for me. This code was added to
2.5 in bug #22764. Not sure what was doing this job before (maybe code removed
by bug #22765?)

On the client, there's code in handle_unit_packet_common() which attempts to
link cargo units to a newly discovered transport, but it has extra checks
which prevent it firing for a player's own transport:


    /* Check if we should link cargo units. */
    if (client_has_player()
        && unit_owner(punit) != client_player()
        && punit->client.occupied) {
      unit_list_iterate(unit_tile(punit)->units, aunit) {
        if (aunit->client.transported_by == punit->id) {
          fc_assert(aunit->transporter == NULL);
          unit_transport_load(aunit, punit, TRUE);
        }
      } unit_list_iterate_end;
    }


Probably this check should just be removed (and the client_has_player() one
too?) Not tested this.

At the moment, if the client learns of a player's cargo before its transport,
code further down in handle_unit_packet_common() lets it through:


  /* Check if we have to load the unit on a transporter. */
  if (punit->client.transported_by != -1) {
    struct unit *ptrans
      = game_unit_by_number(packet_unit->client.transported_by);

    /* Load unit only if transporter is known by the client. For full
     * unit info the transporter should be known. See recursive sending
     * of transporter information in send_unit_info_to_onlookers(). */
    if (ptrans && ptrans != unit_transport_get(punit)) {
      /* ... */
  }


(The comment about send_unit_info_to_onlookers() is now wrong.)

Maybe there should be a client-side sanity check for consistency between known
cargo and transports at some suitable point? Not sure if client-side sanity
checks make sense.

I think this is most likely to affect reloading from savegames, for cargo
units created later than their transport (as the server naturally sends units
in reverse order of original creation).

    _______________________________________________________

Reply to this item at:

  <http://gna.org/bugs/?23444>

_______________________________________________
  Message sent via/by Gna!
  http://gna.org/


_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to