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

This subforum contains all the issues which we already resolved.
Post Reply
sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

"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.

Rseding91
Factorio Staff
Factorio Staff
Posts: 13171
Joined: Wed Jun 11, 2014 5:23 am
Contact:

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

Post by Rseding91 »

Thanks for noticing. It's now fixed for the next version of 0.15.
If you want to get ahold of me I'm almost always on Discord.

sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

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".

Rseding91
Factorio Staff
Factorio Staff
Posts: 13171
Joined: Wed Jun 11, 2014 5:23 am
Contact:

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

Post by Rseding91 »

Making it return what the docs say so "nil" or the entity for underground belts.
If you want to get ahold of me I'm almost always on Discord.

dehook
Manual Inserter
Manual Inserter
Posts: 3
Joined: Sun Jan 15, 2017 1:59 pm
Contact:

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

Post by dehook »

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.

sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

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.

Rseding91
Factorio Staff
Factorio Staff
Posts: 13171
Joined: Wed Jun 11, 2014 5:23 am
Contact:

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

Post by Rseding91 »

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.
If you want to get ahold of me I'm almost always on Discord.

User avatar
Klonan
Factorio Staff
Factorio Staff
Posts: 5148
Joined: Sun Jan 11, 2015 2:09 pm
Contact:

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

Post by Klonan »

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

dehook
Manual Inserter
Manual Inserter
Posts: 3
Joined: Sun Jan 15, 2017 1:59 pm
Contact:

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

Post by dehook »

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.

gheift
Fast Inserter
Fast Inserter
Posts: 188
Joined: Tue Mar 03, 2015 9:20 pm
Contact:

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

Post by gheift »

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

Code: Select all

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

sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

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

Orphan Finder
Fast Replace Underground Belts

sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

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

User avatar
Optera
Smart Inserter
Smart Inserter
Posts: 2915
Joined: Sat Jun 11, 2016 6:41 am
Contact:

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

Post by Optera »

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]

Nexela
Smart Inserter
Smart Inserter
Posts: 1828
Joined: Wed May 25, 2016 11:09 am
Contact:

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

Post by Nexela »

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 = {}}

User avatar
Narc
Filter Inserter
Filter Inserter
Posts: 278
Joined: Mon Feb 11, 2013 7:25 am
Contact:

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

Post by Narc »

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

Nexela
Smart Inserter
Smart Inserter
Posts: 1828
Joined: Wed May 25, 2016 11:09 am
Contact:

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

Post by Nexela »

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

User avatar
Narc
Filter Inserter
Filter Inserter
Posts: 278
Joined: Mon Feb 11, 2013 7:25 am
Contact:

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

Post by Narc »

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.

Nexela
Smart Inserter
Smart Inserter
Posts: 1828
Joined: Wed May 25, 2016 11:09 am
Contact:

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

Post by Nexela »

I miss next(neighbours) already :)

User avatar
Optera
Smart Inserter
Smart Inserter
Posts: 2915
Joined: Sat Jun 11, 2016 6:41 am
Contact:

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

Post by Optera »

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.

sparr
Smart Inserter
Smart Inserter
Posts: 1327
Joined: Fri Feb 14, 2014 5:52 pm
Contact:

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

Post by sparr »

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

Post Reply

Return to “Resolved Problems and Bugs”