[1.1.59] Writing to entity.direction modifies find_entities_filtered iteration order

[Therax]

My mod expects the order of entities in the same position to be consistent when querying find_entity() and find_entities_filtered() as long as no entities are created or destroyed. Starting with version 1.1.58, this is no longer the case, and modifications to an entity's direction will change its iteration order.

Repro1:

1. Start a new map.
2. Create two entities in the same chunk.

Code: Select all

/c game.surfaces.nauvis.create_entity{name="inserter",position={0,0}} game.surfaces.nauvis.create_entity{name="inserter",position={1,0}}

3. Find these entities and note the iteration order.

Code: Select all

/c es=game.surfaces.nauvis.find_entities_filtered{name="inserter",area={{0,0},{2,2}}} for i=1,#es do game.print(es[i].unit_number) end

4. Perform a no-op write to the second entity in iteration order.

Code: Select all

/c es[2].direction=es[2].direction

5. Check iteration order again.

Code: Select all

/c es=game.surfaces.nauvis.find_entities_filtered{name="inserter",area={{0,0},{2,2}}} for i=1,#es do game.print(es[i].unit_number) end

Expected: iteration order is the same. This is the case in versions <= 1.1.57.
Actual: the entity whose direction field was written to becomes the first entity in iteration order. (Confirm by testing with many entities in the same position.)

Note that scripting is not necessary for any operations other than checking the iteration order. The inserters can be built by hand, and then the correct one (second in iteration order) rotated by the normal keyboard shortcut.

When there are more entities involved, the modified (whether by actual rotation or writing to direction) entity becomes the first in the new iteration order.

Alternate repro2:

1. Start a new map.
2. Create multiple entities in the same position:

Code: Select all

/c for i=1,2 do game.player.surface.create_entity{position={2,0},force='player',name='storage-tank'} end

3. Display iteration order of entities:

Code: Select all

/c local s=game.player.selected; for _, e in pairs(s.surface.find_entities_filtered{position=s.position}) do game.print(e.unit_number) end

4. Write to direction of the second entity:

Code: Select all

/c local s=game.player.selected; es=s.surface.find_entities_filtered{position=s.position}; es[2].direction=0

5. Display iteration order of entities again:

Code: Select all

/c local s=game.player.selected; for _, e in pairs(s.surface.find_entities_filtered{position=s.position}) do game.print(e.unit_number) end

Expected: iteration order is the same. This is the case in versions <= 1.1.57.
Actual: the entity whose direction field was written to becomes the first entity in iteration order. (Confirm by testing with many entities in the same position.)

[Therax]

As an additional note, this behavior greatly complicates circuit wire interactions.

Suppose that two circuit-connectable entities are in the same position as part of a compound entity. The player creates a connection to the entity via clicking with a red wire. Then sometime later the player clicks again with a red wire to remove the connection.

Since the selected entity is whichever is currently first in iteration order, and iteration order can now change, this action may remove the circuit wire connection as intended, or create a new circuit wire connection to the other entity in the "stack."

[boskid]

This is not a bug. It is a consequence of how 101611 was fixed: in that case when changing direction of an entity, it is released from the surface and reregistered because it may cover different area of advanced tiles. Order of entities returned by find_entities_filtered across all clients is guaranteed to be the same but there are no guarantees given for what operations will change or not change the order of entities returned. Every time an entity is released from the surface and reregistered, it will appear on the front of the list on all advanced tiles that collide with entity's bounding box and this directly affects the order of returning entities. Same behavior happens for objects that are moving (flying robots, character, all vehicles, units) or in general when entity is teleported around.

Issues related to player selecting entities should not be fixed by means of trying to create entities in a certain order. You should be able to solve selection issues with EntityPrototype::selection_priority instead.

[Therax]

If would be nice if the unregister and reregister logic for 101611 only executed if the entity being rotated actually had an off-center collision box, saving a bunch of redundant work in the 99% case.

[boskid]

If would be nice if the unregister and reregister logic for 101611 only executed if the entity being rotated actually had an off-center collision box, saving a bunch of redundant work in the 99% case.

But that would be more work to detect if an entity changes the advanced tiles it should be registered on as it would require finding the entity's AABB before and after the operation to only release and reregister the entity when the AABB before and after are different. This could also be implemented in general but then it would fail in general because the order of returned entities could still change from version to version, for example in 1.1 entity searches are performed by advanced tiles row by row (top to bottom, from left to right) and entities are added to the result when they are seen for the first time during the search (top-most left-most common advanced tile between search area and entity area) while in 1.2 entities will not be guaranteed to be returned when they are first seen but when seeing them on the advanced tile which is the closest one to the advanced tile position related to entity's center position. Entity moving 0.1 tile to the right could change its center position to be on a different advanced tile causing the order to be different while the area of advanced tiles would not change at all. Its also not possible to restore the order when reregistering entity on the surface as there must be a deterministic global order of entities because it affects order of saving those entities in a way when loading them will reregister them on the advanced tiles during load to exactly the same order as they were before. This step is required exactly for entity searches to have deterministic order between all clients and it means the easiest way to reregister entity is to always put it in the front of all advanced tiles. Trying to insert somewhere in the middle of list would be more expensive and i can easily craft an example with 2x2 entities where such operation would need to visit literally every entity on a surface to decide where the entity should be put in every list.