Factorio Forums

Posted: **Sun May 21, 2017 2:12 am**

"When called on an underground transport belt, this is the other end of the underground belt connection, or nil if none."

This changed at some point and now seems to return {} instead of nil for an unconnected underground input. Probably should just be fixed in the docs.

Posted: **Mon May 22, 2017 1:23 am**

Thanks for noticing. It's now fixed for the next version of 0.15.

Posted: **Mon May 22, 2017 1:47 am**

Can you say if you're updating the docs or switching the code back from {} to nil?

I already updated one mod to support either option, but I'd prefer to update others "correctly".

Posted: **Mon May 22, 2017 2:12 am**

Making it return what the docs say so "nil" or the entity for underground belts.

Posted: **Tue May 23, 2017 7:31 pm**

Regardless of what the docs say, returning an empty collection generally allows for more concise client code than returning nil. This change has also broken at least two mods that used that more concise style.

Posted: **Tue May 23, 2017 9:38 pm**

How long was this bug in effect? If it happened in maybe 0.15.9 then reverting it makes sense. If it happened in 0.14.0 and no one noticed until now but multiple mods have been written in the last year that use the {} behavior, then maybe {} is the better option.

Posted: **Tue May 23, 2017 10:26 pm**

The electric pole result being a dictionary already means the return result must be inspected before it can be meaningfully used.

I don't really mind either way but it doesn't seem like one format or the other gives any advantage.

Posted: **Tue May 23, 2017 10:53 pm**

dehook wrote:Regardless of what the docs say, returning an empty collection generally allows for more concise client code than returning nil. This change has also broken at least two mods that used that more concise style.

I find that returning nil is generally far easier to work with,

Code: Select all

if entity.neighbors then ...

is much simpler than

Code: Select all

if entity.neighbors[1] then...

Rseding91 wrote:The electric pole result being a dictionary already means the return result must be inspected before it can be meaningfully used.

I don't really mind either way but it doesn't seem like one format or the other gives any advantage.

I would keep it as nil, as it is more consistent with the way most of the other API calls work

Posted: **Tue May 23, 2017 11:30 pm**

Klonan wrote: I find that returning nil is generally far easier to work with,

The savings comes in something like this from an effected mod:

Code: Select all

  for _, neighbour in pairs(entity.neighbours) do...

If entity.neighbours is an empty collection, then the for loop just doesn't do anything with no check for nil or empty required.

Posted: **Wed May 24, 2017 1:29 am**

When it returns nil, just expand the code like this:

Code: Select all

for _, neighbour in pairs(entity.neighbours or {}) do...

Posted: **Wed May 24, 2017 4:38 am**

Apparently this bug existed for a while. Changing it back broke the following mods for me:

Orphan Finder
Fast Replace Underground Belts

Posted: **Wed May 24, 2017 4:55 am**

Code: Select all

local function has_neighbours(entity)
  return not (entity == nil or entity.neighbours == nil or (#entity.neighbours == 0 and not entity.valid))
end

Posted: **Wed May 24, 2017 6:13 am**

Directly returning the other ug belt entity seems a bit counter intuitive to the the description.

neighbours :: dictionary string → array of LuaEntity or array of LuaEntity or LuaEntity [Read-only]

I would expect a dictionary containing the entity instead.

It should be:
neighbours :: → array of LuaEntity or array of LuaEntity or LuaEntity [Read-only]

Posted: **Wed May 24, 2017 8:26 am**

I agree this "change" leads to some ugly hacks The way I thought of neighbours before this change was "Hey I want to visit the next(neighbours)
The plural s on neighbours makes my mind thing it is going to return a table with 0 or more items

before when using it on both pipes and undergrounds

if neighbours and #neighbours < 1 then ------

now I have to do some wonky stuff like compare types

if not neighbours or neighbours and not neighbours.type and #neighbours < 1 then -----

@optera
dictionary string → array of LuaEntity is for electrical poles {red = {}, copper = {}, green = {}}

Posted: **Wed May 24, 2017 10:32 am**

Nexela wrote:I agree this "change" leads to some ugly hacks [...] before when using it on both pipes and undergrounds

if neighbours and #neighbours < 1 then

Shouldn't that still work? I mean, 'if not neighbours' will be true if `if neighbours == nil` is true.

sparr wrote:

Code: Select all

local function has_neighbours(entity)
  return not (entity == nil or entity.neighbours == nil or (#entity.neighbours == 0 and not entity.valid))
end

I always like my sanity checks separate from the actual logic, so

Code: Select all

local function has_neighbours(entity)
    if not entity or not entity.valid then return false end
    if not entity.neighbours then return false end

    return entity.neighbours < 1
end

Posted: **Wed May 24, 2017 10:57 am**

Narc wrote:

Code: Select all

    local function has_neighbours(entity)
        if not entity or not entity.valid then return false end
        if not entity.neighbours then return false end

        return entity.neighbours < 1
    end

assuming your return is just missing the #
on an underground belt that will return 0 if the belt has neighbours since the neighbour is an entity

Posted: **Wed May 24, 2017 11:17 am**

Nexela wrote:assuming your return is just missing the #

Yup, sorry, untested code. It's also got the logic backward, and would fit "has_no_neighbours(entity)"

Nexela wrote:on an underground belt that will return 0 if the belt has neighbours since the neighbour is an entity [and entities have 0 array members, but still answer to #table]

Oh, yuck, I see what you mean. Might be a dictionary, might be an array, might be a single entity depending on the type. In fact, let's not forget: might raise an error if used on an entity that doesn't support it.

Code: Select all

local function has_neighbours(entity)
    if not entity or not entity.valid then return false end
    if not entity.neighbours then return false end

    if entity.type == "underground-belt" then
        return entity.neighbours and entity.neighbours.valid
    elseif entity.type == "electric-pole" then
        return #entity.neighbours.copper > 0 or #entity.neighbours.red > 0 or #entity.neighbours.green > 0
    else
        return #entity.neighbours > 0
    end
end

Oh, and this will still raise an error if used on an entity that doesn't support .neighbours at all. I just don't even at this point.

Posted: **Wed May 24, 2017 2:21 pm**

I miss next(neighbours) already

Posted: **Wed May 24, 2017 2:37 pm**

sanity check for ug belts actually is really simplistic now:

Code: Select all

if entity.type == "underground-belt" and entity.neighbours then

My problem is like Nexela said, neighbours is plural indicating an array.
Also it returns 3 different things for the 3 different entity types it can be used on.

Posted: **Wed May 24, 2017 10:48 pm**

adding my opinion to the side that "neighbourS" ever returning a single entity, rather than a 1-entity array, makes for ugly code.

Factorio Forums

[0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

[0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil

Re: [0.15] [Rseding91] LuaEntity:neighbours on underground belt returns {} not nil