raiguard / EditorExtensions

Extends the Factorio map editor with new features, testing tools, and other utilities to aid with sandbox play and scenario creation.
https://mods.factorio.com/mod/EditorExtensions
Other
26 stars 6 forks source link

Crash when copy-pasting settings in infinity accumulator #146

Closed TobTobXX closed 12 months ago

TobTobXX commented 1 year ago

Description

This is the error that pops up:

The mod Editor Extensions (2.2.0) caused a non-recoverable error.
Please report this error to the mod author.

Error while running event EditorExtensions::on_entity_settings_pasted (ID 33)
LuaEntity API call when LuaEntity was invalid.
stack traceback:
    [C]: in function '__index'
    __EditorExtensions__/scripts/infinity-loader.lua:150: in function 'handler'
    __core__/lualib/event_handler.lua:47: in function <__core__/lualib/event_handler.lua:45>

Factorio version: 1.1.87 Editor extension version: 2.2.0 flib version: 0.12.9

Reproduction

  1. Open an empty lab world (and delete the preexisting structures)
  2. Place two default Infinity Accumulators
  3. Switch one to "Buffer" mode.
  4. Copy the settings of the buffer acumulator using shift-right click.
  5. Paste into the other accumulator using shift-left click.
  6. Enjoy the beautiful error pop-up.
TobTobXX commented 1 year ago

If you give me a week of time, I'll probably also come up with a PR to fix it. I'd love to try my hand at it, as I'm just getting started in mod dev.

raiguard commented 12 months ago

Thanks for the offer, but fortunately this is a pretty simple fix.

TobTobXX commented 12 months ago

I tried to find the error, but I just don't get it.

Something very weird is happening, that we land in the event handler for the infinity loader, not the infinity accumulator. Also the error claims that the LuaEntity is invalid, but I just can't see where it was invalidated, considering I copied the settings after I switched it to buffer mode.

Please, go ahead and fix it. But link the commit here, I'm very interested in what was wrong. Anyway, I learned a lot.

raiguard commented 12 months ago

This happened because I'm using the event_handler library, which lets you specify multiple handlers to be run for a given event. The entity was invalidated when the handler in infinity-accumulator.lua ran, so the handler in infinity-loader.lua crashed because I forgot to add the validity check. event_handler does not abort when something is invalid because always checking would hurt performance, and it's easy to do on the mod's side.

TobTobXX commented 12 months ago

Ah, I see. I somehow assumed event handlers would be registered on an entity/surface/whatever, but events are global and all handlers for on_settings_pasted will receive the event.

If I may ask, why are you using event_handler.add_libraries() instead of game.on_event()? Is there a difference between these two interfaces, or is it just because it makes the code look how you want?

raiguard commented 12 months ago

event_handler is a wrapper over script.on_event. script.on_event only supports one handler per event, so event_library exists to allow multiple handlers to exist. It just calls script.on_event and gives it a function that loops over all the functions you specified and calls them.