[1.1.42] LuaSurface::clone_brush() moves trains that are stopping with wagons on a bend

[Harag]

Issue

The train facing up in this layout is behind the train stop after it is cloned two times via LuaSurface::clone_brush resulting in "no path" after it is switched back to automatic mode. This is because each clone slightly moves the train upwards.

How to reproduce

Use this savegame and this script in the editor's "Run Lua" tool on the area with concrete underneath:

Code: Select all

local surface = game.player.surface

local sx, sy = unpack(area.left_top)
local ex, ey = unpack(area.right_bottom)
local dx = ex - sx + 10

local loco = unpack(surface.find_entities_filtered { area = area, name = "locomotive" })
if loco then
    game.print(serpent.line(loco.position))
    loco.train.manual_mode = false
end

for i=1,2 do
    local clone_positions = {}

    for _, tile in pairs(surface.find_tiles_filtered {
        area = { {sx, sy,}, {ex, ey}},
        name = "refined-concrete",
    }) do
        if tile.name == "refined-concrete" then
            table.insert(clone_positions, tile.position)
        end
    end

    game.player.surface.clone_brush {
        source_offset = { x = 0, y = 0 },
        source_positions = clone_positions,
        destination_surface = surface,
        destination_offset = { x = dx,  y = 0},
        clone_tiles = true,
        clone_entities = true,
        expand_map = true,
    }

    local dest = { { sx + dx, sy}, { ex + dx, ey }}
    local loco = unpack(surface.find_entities_filtered { area = dest, name = "locomotive" })
    if loco then
        game.print(serpent.line(loco.position))
        loco.train.manual_mode = false
    end

    -- clone a 2nd time
    sx = sx + dx
    ex = ex + dx
end

Notes

• Only clone_brush() does this. clone_area() clones the trains perfectly
• The train facing down is also affected but also moves up, so it stays in front of the train stop
• Cloning trains stopped on a completely straight track is fine
• This is not an academic issue, Space Exploration clones spaceships (entities on spaceship floor-tiles) via clone_brush() at least two times - from planet-surface to travel-surface and from travel-surface to destination planet-surface - and a ship might make waystops before finally releasing trains on-board (so 2*n clones)
• It is deseriable to have that bend before the stop because spaceships have a tile-limit, which requires to design them as compact as possible
  
  

Thoughts

The position error accumulates with each clone so it would be best if clone_brush() made at least sure that the leading locomotive stays in place (but what is "leading" with double-ended trains?). It might be easier to be more lenient when switching a train from manual to automatic and checking if it is already at the currently scheduled train stop.

EDIT: The locomotive that should stay put is decidable from the waiting state of the train. It's the one next to the stop the train is waiting at. As long as the caller of clone_brush() restores automatic mode after cloning this should propagate correctly.

[boskid]

Oh no, trains clone :/

This issue is really annoying. In case of rolling stocks their primary source of specifying position and orientation are 2 joints (front and back) while the rolling stock position and orientation are only computed from that. Clone tries to create new rolling stocks at a given position and orientation but to do that it has to find a pair of joint positions that gives a closest possible position. In order to properly fix this i would have to make a dedicated rolling stock creation mode when both joints are specified assuming the rail they are on are included in the clone operation (it would not work when doing a train only clone or when a clone would cover a rolling stock and rails under one of those joints). Other solutions are merely a workaround like trying to move rolling stocks slightly to maybe make them closer to where they should be.

clone_brush vs clone_area behavior internally is exactly the same as they both call Surface::cloneEntities to perform clone of entities. Differences you are seeing are due to entity clone order where in the clone_area all rolling stocks are cloned from top to bottom (due to entity search happening on the specified area to find entities to be cloned), while with the clone brush the order of clone will be simillar to the order of source_positions as the entities are collected using N entity searches over multiple tiles. In your case those positions are based on LuaSurface::find_tiles_filtered which scans the area from left to right so the order of tiles is also left-to-right and the order of the entity clone will also be left-to-right making the train to be cloned in the opposite direction starting from the rolling stock that is on curved rail. All other rolling stocks cloned after are snapped to the already cloned rolling stock so their position does not matter that much.

[Harag]

So a workaround could be to reorder source_positions to make sure the rolling stock of a train in state WAIT_STATION that is next to a train stop is cloned first? Would clone_brush get upset if source_positions contains duplicates? That would allow to go in two passes. First find the positions of important rolling stock then find all relevant tiles for the clone - EDIT: well I can simply try it out

[boskid]

Would clone_brush get upset if source_positions contains duplicates? That would allow to go in two passes. First find the positions of important rolling stock then find all relevant tiles for the clone - EDIT: well I can simply try it out

No it wont. Entities that were already collected during entity search on one source position are ignored by all the remaining entity searches on other source positions - an entity is only collected once.
Reordering source positions is a really ugly hack as it is not guaranteed to work and it may break with any changes to internal stuff.

[Harag]

I'm just trying to think of an interim solution I can suggest to the SE developers. The proper solution sounds like a larger effort.

EDIT: I played with this a little and getting the rolling stock next to the stop is doable. What's not exactly easy is to get only the tiles under it. Even in my small example above the downward-facing locomotive is slightly rotated and has a wagon right next to it that will cause the initial problem when cloned first. "Ugly" indeed.

[boskid]

Thanks for the report however i am going to move it to Not a bug. As mentioned already, clone of rolling stocks uses rolling stock position and orientation however those are secondary sources of data, for a proper clone i would need to completly rewrite how clone of rolling stocks works so it would copy them by their wheels positions instead which would require some new checks to be created to verify if target clone positions have correct rails between wheels (partial clone of rolling stock + rail in front and in the back but missing rail in the middle would have to be disallowed). I am not doing that.