Incorrect behaviour for "Collect all astrounauts" command

Ask here if you experience technical problems with X³: Terran Conflict, X³: Albion Prelude or X³: Farnham's Legacy.

Moderators: timon37, Moderators for English X Forum

Post Reply
azaghal
Posts: 94
Joined: Wed, 21. Mar 07, 13:19
xr

Incorrect behaviour for "Collect all astrounauts" command

Post by azaghal » Thu, 7. Nov 19, 21:29

Small preamble

Please be a bit patient, I've tried to dig into the relevant script code to find the issue, so it is a bit of a wall of text here :)


Environment information

Game: X3AP 3.1a, English (gog_x3_terran_war_pack_2.4.0.7.sh)
Store: GOG
Operating system: Debian 9.11 (with latest kernel from backports)
Modified: yes (but please see the additional information section, there is explicit information on what scripts are broken and in what way)
Game type: n/a

Modded information

The problem I faced was specifically found while playing the Litcube Universe mod, but I have noticed that the same bug in the script is present in vanilla as well. E.g. the relevant script logic is simply incorrect.


Reproduction steps
  1. Purchase (for example) cheapest variant of Argon Discoverer.
  2. Apply maximum Cargo Bay Extension, Engine Tuning, and Rudder Optimisation upgrades.
  3. Equip ship with the following equipment: Cargo Lifesupport System, Special Command Software MK1, Transporter Device.
  4. Unlock script console.
  5. Spawn a couple of astronauts via script console throughout the sector where the Argon Discoverer is located.
  6. Order the Argon Discoverer to collect all astronauts (Special -> Collect Astronaut -> Collect all astronauts).

Expected results
  1. Argon Discoverer proceeds to collect all astronauts in optimal manner (e.g. picking up nearest astronauts first).
  2. Argon Discoverer consistently picks up all astronauts in the sector provided there is sufficient room in the cargo bay.

Actual results
  1. Argon Discoverer seems to select the next Astronaut to pick-up completely randomly, resulting in flying all over the sector.
  2. Argon Discoverer may refuse to pick up Astronauts depending on how full its cargo bay is - even if there is sufficient free space available.

Additional information

The listed issues are related to incorrect logic within the two following files:
  • !ship.cmd.collectastronaut.pl.pck
  • !move.collectware.pck
The issue with the suboptimal path-finding can be traced to the code in the !ship.cmd.collectastronaut.pl.pck script:
  1. First we have a snippet that tries to locate the nearest astronaut to pick-up:

    Code: Select all

    $flags = [Find.Multiple]
    $ships = find ship: sector=[SECTOR] class or type=[Astronaut] race=null flags=$flags refobj=[THIS] maxdist=null maxnum=100 refpos=null
    
    $wareobj = null
    if $ships
        for each $wareobj in array $ships using counter $count
            * Remove Player SpaceSuit
            if $wareobj == [PLAYERSHIP]
                remove element from array $ships at index $count
            else
                * remove non marines from list
                if $marineOnly
                    skip if is marine: passenger/astronaut=$wareobj
                        remove element from array $ships at index $count
                end
            end
        end
        
        * find closest
        $wareobj = null
        $nearest = null
        $nearest.dist = 2147483647
        
        for each $wareobj in array $ships using counter $count
            $dist = get distance between [THIS] and $wareobj
            if $dist < $nearest.dist
                $nearest = $wareobj
                $nearest.dist = $dist
            end
        end
    end
    
  2. Take note that the nearest astronaut object is stored in the variable $nearest.
  3. Then we have the second relevant part which actually commands the ship to pick-up the astronaut:

    Code: Select all

    if $wareobj-> exists
        [THIS]-> set command: [COMMAND_COLLECT_ASTRONAUT]  target=$wareobj target2=null par1=null par2=null
        = [THIS]-> call script '!move.collectware' : argument1=$wareobj argument2=[FALSE]
    else
        [THIS]-> set command: [COMMAND_COLLECT_ASTRONAUT]  target=null target2=null par1=null par2=null
        = [THIS]-> move around 60000 ms
    end
    
  4. Take note that this snippet does not use the $nearest variable, but instead relies on the $wareobj variable - which results in the commanded ship to pick-up different "random" astronaut every single time depending on what the initial $ships array gets populated with. based on what I can tell from this, this second snippet should instead look like as follows:

    Code: Select all

    if $nearest-> exists
        [THIS]-> set command: [COMMAND_COLLECT_ASTRONAUT]  target=$nearest target2=null par1=null par2=null
        = [THIS]-> call script '!move.collectware' : argument1=$nearest argument2=[FALSE]
    else
        [THIS]-> set command: [COMMAND_COLLECT_ASTRONAUT]  target=null target2=null par1=null par2=null
        = [THIS]-> move around 60000 ms
    end
    
The issue with the commanded ship being unable to pick-up the astronauts even though there is sufficient room seems to come from the code in !move.collectware.pck:
  1. The main issue lies within the following snippet:

    Code: Select all

    if not $destroy OR $is.astronaut
        $ware = $wareobj-> get ware type code of object
        skip if [THIS]-> can transport ware $ware
            return null
        $free = [THIS]-> get free amount of ware $ware in cargo bay
        skip if $free
            return null
    end
    
  2. The above code is a bit deceptive because of the use of if not construct. In most programming languages the above would be interpreted similar to (emphasis with brackets for ordering) if ((not $destroy) OR ($is.astronaut)). E.g. first you evaluate the not $destroy expression, then $is.astronaut expression, then you apply the OR operator. However, if I understood this well, the if not construct in X script editor is a keyword on its own. E.g. it'd be more appropriate to spell it as ifnot. E.g. what happens is that (do correct me if I'm wrong) the interpreter will evaluate the expression $destroy OR $is.astronaut, and then apply the negation (ifnot) over the result of that operation.
  3. So, what happens is essentially that the $destroy OR $is.astronaut in this particular case gets evaulted to TRUE, which then gets negated by ifnot, and that little snippet is never executed.
  4. Later on in the script we have the following:

    Code: Select all

    if $doneStop
    
        skip if [THIS]-> can transport ware $ware
            return null
        $free = [THIS]-> get free amount of ware $ware in cargo bay
        skip if $free
            return null
        
        if [THIS]-> get amount of ware {Transporter Device} in cargo bay
        
            if $is.astronaut
                $catchedamount = [THIS]-> catch ware object $wareobj
            else
                * $Amount = $wareobj-> get flying ware count
                * $WareType = $wareobj-> get ware type code of object
                * = [THIS]-> add $Amount units of $WareType
                * $wareobj-> destruct: show no explosion=[FALSE]
                $catchedamount = [THIS]-> catch ware object $wareobj
            end
            return $catchedamount
        end
    end
    
  5. Here we have a small check for presence of transporter device and whether the ware is an astronaut before we attempt to pick it up. But... At the very top we first check if the ware ($ware) can be transported. But, as mentioned earlier, in case of an $is.astronaut being TRUE and incorrect use of if not expression, the $ware variable will not be set, resulting in the can transport ware and get free amount of ware working with a null.
  6. Now, if I remember correctly (it has been a couple of days, and I just continued playing with applied fix, so my apologies on this :) ), the get free amount of ware used to return TRUE if the base cargo limit was not reached if you pass in a NULL ware to it. So, if your ship had not filled-up the base cargo limit (the upgrades did not matter), it'd happily pick-up the astronaut. However, if the base cargo limit was already exceeded, it'd just barf out at that point. I guess a bit of undefined beahviour in a way.
  7. To fix this, the first snippet should like as follows (maybe this expression could be written in nicer way):

    Code: Select all

    if $destroy == [FALSE] OR $is.astronaut
        $ware = $wareobj-> get ware type code of object
        skip if [THIS]-> can transport ware $ware
            return null
        $free = [THIS]-> get free amount of ware $ware in cargo bay
        skip if $free
            return null
    end
    

Sparky Sparkycorp
Moderator (English)
Moderator (English)
Posts: 8074
Joined: Tue, 30. Mar 04, 12:28
x4

Re: Incorrect behaviour for "Collect all astrounauts" command

Post by Sparky Sparkycorp » Fri, 8. Nov 19, 00:51

Thank you for this detailed and interesting (!) report :)

I'm not fimilar with coding so please forgive the following naive question.

Does rmship radar range have any impact on your description of the reproduction steps?

azaghal
Posts: 94
Joined: Wed, 21. Mar 07, 13:19
xr

Re: Incorrect behaviour for "Collect all astrounauts" command

Post by azaghal » Fri, 8. Nov 19, 12:24

Hey, glad someone actually decided to read it :)

The radar range did not make any impact. In original testing I've used a ship with triplex scanner equipped as well, and the sector was fully covered by advanced navigation satellites (e.g. I could see very well the positioning of astronauts).

For example, even if you had 3-4 astronauts sitting next to each-other, the ship would pick up one of them, and then very often try to head out to the opposite side of sector to pick up the next one (depending on how the find function would order the astronauts).

With the suggested code changes the astronauts were picked-up reliably 100% of the time (unless they managed to escape to a station prior to that :) ).

As a side note, from what I can tell, the code does not actually try to accommodate for radar range in any way, since it uses just generic functions for locating astronauts.

Post Reply

Return to “X³: Terran Conflict / Albion Prelude / Farnham's Legacy - Technical Support”