undo triggers on_built_entity, can't tell if entity was made by undo

[bobingabout]

In my Adjustable inserters mod, I use on_built_entity to test if the player has built an entity, and given quite a few tests (to say, apply to a ghost, but not if it was put there by a blueprint, by checking if the player has a blueprint or book in their hand), apply the changed hand positions where appropriate.
However, since the rather new feature "Undo" has been added, which also triggers this same event, If someone does perform an undo to replace an inserter while my position overrides are active, it applies to that restored inserter ghost.
Since there is no easy way to directly test if the thing was created by an undo condition or not, I'm a bit at a loss trying to check if this is true. I managed to filter to if the player is specifically holding an item, but... it's possible the player may be holding a ghost of the item, where this check falls through.

So, the request:

Either move the undo action to it's own event, or add a new true/false check for an undo action on the existing on_built_entity event.

[eradicator]

Generally doesn't sound like a bad idea but i'm having trouble understanding what exact order of events you're having problems with. Btw if the player holds a ghost you can read that via LuaControl.cursor_ghost.

[Rseding91]

This feels... off. Why exactly do you care how a ghost came to exist?

[bobingabout]

Generally doesn't sound like a bad idea but i'm having trouble understanding what exact order of events you're having problems with. Btw if the player holds a ghost you can read that via LuaControl.cursor_ghost.

Even if I were to specifically limit setting the hand positions to "Player is holding the item or ghost that places the newly created ghost" (Which I'm not entirely sure how to do, I can't see a hook in item_stack to check what the placed entity type and name would be, just matching item and entity name isn't enough as they may be different), you'd still get the issue where if someone was holding an inserter of the type that they had just mined, and then pressed the undo button, the condition would be met because the player is holding the correct item.

[Rseding91]

https://lua-api.factorio.com/latest/Lua ... .prototype to read the item's prototype and then https://lua-api.factorio.com/latest/Lua ... ace_result to read what it builds (if anything).

[bobingabout]

This feels... off. Why exactly do you care how a ghost came to exist?

Because my Adjustable inserters mod gives the player the option to place an inserter, or a ghost of an inserter, and apply the pre-selected hand positions to it.
Pressing undo currently sets those positions to any restored inserters, which might be wrong.
Even if I were to specifically limit the settings being applied to if the player is holding an inserter, they may still press the undo button while holding an inserter, which would then apply the settings anyway.

Personally, I don't go around leaving the "Apply these hand positions to newly placed inserters" option turned on... but I've been getting bug reports that it's an issue, and its frustrating that I can't fix every possible hole in the logic.

EDIT:

https://lua-api.factorio.com/latest/Lua ... ace_result

that's the one I was looking for, but was failing to find.

Still, there's the possibillity that some player may be holding that item when they press undo, which I currently can't filter for.

[eradicator]

Hm... one way would be to hook into the undo keypress via a linked custom input. Though you'd miss when the player clicks the shortcut button instead...

So let's see how ghosts come into existance:

Player places a blueprint -> cursor_stack.type == 'blueprint'
Mod places a blueprint via LuaItemStack.build_blueprint -> event.item.name == 'blueprint'
As above but without by_player -> script_raised_built

Player places one ghost -> cursor_stack.name or cursor_ghost.name == 'your-inserter' and event.item.name == 'your-inserter'
Mod uses LuaSurface.create_entity -> script_raised_built
As above but without raise_built -> no event

Player does undo -> any event left after exclusion of the above... (also event.item == nil)

Convoluted but possible?

[bobingabout]

Hm... one way would be to hook into the undo keypress via a linked custom input. Though you'd miss when the player clicks the shortcut button instead...

So let's see how ghosts come into existance:

Player places a blueprint -> cursor_stack.type == 'blueprint'
Mod places a blueprint via LuaItemStack.build_blueprint -> event.item.name == 'blueprint'
As above but without by_player -> script_raised_built

Player places one ghost -> cursor_stack.name or cursor_ghost.name == 'your-inserter' and event.item.name == 'your-inserter'
Mod uses LuaSurface.create_entity -> script_raised_built
As above but without raise_built -> no event

Player does undo -> any event left after exclusion of the above... (also event.item == nil)

Convoluted but possible?

Currently, with the help of you and Rseding.
in the on_built_entity event: (This specifically checks if the player did it, which is the only situation we're interested in.)
First, we check to see if the hand position setter is turned on, if it isn't we stop, if it is we continue.
Then we check to see what is in the player's hand, if it is an item, or ghost, that places an entity of type "inserter" we continue.
we then check to see what was built, if it was an entity of type inserter, or a ghost of type inserter we continue.
if hand_item_place_result.name == entity_name (The item in your hand can place the entity created), we continue.
We also check if the entity is on the blacklist, and if entity.revived, if either is true, we drop out.

And if all these checks pass... we apply the new hand positions.

This works as intended in every case (Applying if you're holding the item or it's ghost, to both a new entity or a new ghost. not applying if you're holding a blueprint, etc) EXCEPT if you press Undo, while holding the item that builds the entity that was restored, in which case it applies the new hand positions when it shouldn't.

I mean, we have entity.revived to specifically check if the ghost was revived into an entity... Why can't we have a check for if the ghost was created via Undo?

[eradicator]

My idea was to apply first-order logic, but for that i'd need a complete table of desired states, including the positive ones. I.e. any action that could possibly create an inserter or ghost must be mapped to either "do" or "don't". Failing to account for even one possible action would render the whole process moot. And i'm currently not sure i know every situations you *do* want to apply.

Code: Select all

Player builds inserter entity from cursor: yes
Player builds inserter ghost from cursor: yes?
Player builds inserter with blueprint: no?
Player builds inserter ghost with undo: no

Player revives inserter ghost: no?
Robot revives inserter ghost: no?

Mod create_entity's inserter entity: no?
Mod create_entity's inserter ghost: no?

Mod builds inserter ghost with blueprint: no?

Any missing situations?

I mean, we have entity.revived to specifically check if the ghost was revived into an entity...

I can't find any reference in the API to LuaEntity.revieved, are you talking about script_raised_revive?

Why can't we have a check for if the ghost was created via Undo?

I'm not the one against it. Even if my approach works.

[bobingabout]

My idea was to apply first-order logic, but for that i'd need a complete table of desired states, including the positive ones. I.e. any action that could possibly create an inserter or ghost must be mapped to either "do" or "don't". Failing to account for even one possible action would render the whole process moot. And i'm currently not sure i know every situations you *do* want to apply.

Code: Select all

Player builds inserter entity from cursor: yes
Player builds inserter ghost from cursor: yes?
Player builds inserter with blueprint: no?
Player builds inserter ghost with undo: no

Player revives inserter ghost: no?
Robot revives inserter ghost: no?

Mod create_entity's inserter entity: no?
Mod create_entity's inserter ghost: no?

Mod builds inserter ghost with blueprint: no?

Any missing situations?

I mean, we have entity.revived to specifically check if the ghost was revived into an entity...

I can't find any reference in the API to LuaEntity.revieved, are you talking about script_raised_revive?

Why can't we have a check for if the ghost was created via Undo?

I'm not the one against it. Even if my approach works.

revived is one of the values in the event data table of on_built_entity. (or it used to be, that's what I'm checking and it ain't crashing.)
But yes, the only instance where I want it to actually apply the hand positions is when the player clicks to place an inserter, or ghost, intentionally, while the option to change positions is turned on in my GUI menu.
But it's also triggering if the player is holding said item when they press undo.
here is my code:

Code: Select all

script.on_event(defines.events.on_built_entity, function(event)
  local player = game.players[event.player_index]
  local entity = event.created_entity

  local hand_item = nil
  if player.cursor_stack and player.cursor_stack.valid and player.cursor_stack.valid_for_read and player.cursor_stack.prototype and player.cursor_stack.prototype.place_result then
    hand_item = player.cursor_stack.prototype.place_result
  elseif player.cursor_ghost and player.cursor_ghost.valid and player.cursor_ghost.place_result then
    hand_item = player.cursor_ghost.place_result
  end

  if
    hand_item and hand_item.type == "inserter"
    and
    (
      (
        entity.type == "inserter"
        and hand_item.name == entity.name
        and not global.bobmods.inserters.blacklist[entity.name]
        and not event.revived
      )
      or
      (
        entity.type == "entity-ghost" and entity.ghost_type == "inserter"
        and hand_item.name == entity.ghost_name
        and not global.bobmods.inserters.blacklist[entity.ghost_name]
      )
    )
  then
    bobmods.logistics.set_positions(entity, event.player_index)
  end
end)

yes, including the table "bobmods" in global and function name is a bit redundant, but it's a habbit I picked up for making my own vesion of functions in the data phase, and I didn't know all that much about control when I FIRST wrote this.
but even after I realised that, the variable would still exist in a save game, so that save would still have the same variable structure, possibly leaving redundant crap in the save if I changed it, so I've never changed it.

[Rseding91]

"revived" isn't a field in any of the events and from what I can see, it hasn't ever been.

[Deadlock989]

"revived" was made up by Nexela. It was passed in raised build events by mods like Nanobots and Bluebuild and other mods which historically used on_player_built for bot and bot-like building events, which was bad in the first place. It is not part of the API and it is not generated when the real on_entity_built event fires naturally.

Another reason to stop people raising base game events.

[bobingabout]

"revived" was made up by Nexela. It was passed in raised build events by mods like Nanobots and Bluebuild and other mods which historically used on_player_built for bot and bot-like building events, which was bad in the first place. It is not part of the API and it is not generated when the real on_entity_built event fires naturally.

Another reason to stop people raising base game events.

Well... then it's not doing anything when I check for it, and I will delete it from my checks.
I probably had it in there because Nexela asked me to add it at some point, and just totally forgot it wasn't part of the base game.

[Rseding91]

I'm not against the idea of having ways to tell how some event was triggered but I don't know when I would implement it, because it means going over every place those are fired and passing the relevant information.

[bobingabout]

I'm not against the idea of having ways to tell how some event was triggered but I don't know when I would implement it, because it means going over every place those are fired and passing the relevant information.

Thank you for Considering it.
I am aware of how complex the code is, and your aims to keep things optimised and efficient.

[bobingabout]

I'm not against the idea of having ways to tell how some event was triggered but I don't know when I would implement it, because it means going over every place those are fired and passing the relevant information.

It turns out there actually was a solution already, I'm kinda surprised it didn't come up in this topic.

Since late 0.17 (actually, probably more mid 0.17, was the 30somethingth revision), on_built_entity's table contains the value "item", which returns the item prototype used to build the entity (even if it's a ghost item from your hotbar, it returns the item that could be used to create that ghost).
If you press undo, this item is nil, because no item was used to create the ghost!

I had to go and revisit the previous script I wrote which checked player.cursor_stack and player.cursor_ghost(ghost doesn't actually matter for this issue), because if you placed the entity and used up your last inserter item to do so, when it checked what was in your hand, it would say you're not holding an item, and not apply the pickup and drop position changes when it was supposed to.


So yeah, Although I still can't tell if entity was created by pressing undo, I can limit to the check I actually want, was it created by the player using an item (or a ghost).

So the new check is just...

Code: Select all

    event.item and event.item.place_result and event.item.place_result.type == "inserter"
    and (entity.type == "inserter" or (entity.type == "entity-ghost" and entity.ghost_type == "inserter"))
    and event.item.place_result.name == entity_name --probably don't even need this line anymore.
    and not global.bobmods.inserters.blacklist[entity_name]

entity_name is saved first, and is entity.ghost_name if entity.type == "entity-ghost", or entity.name in all over cases.