raise_event() should add .raised = true to table

[aubergine18]

Currently there's no way to distinguish between a vanilla event and a game.raise_event() event.

If raise_event() added a `.raised = true` to the table in second parameter then listeners would always be able to determine if the event came from the game or a mod. Mod-raised events would always have .raised=true

Code: Select all

-- example

script.on_event( defines.events.on_built_entity, function( data )
  if data.raised then
    -- i know a mod did this
  else
    -- it's vanilla event
  end
end

This allows mods to more safely make assumptions about the state of the game.

In addition, it would be nice if the LuaSurface.create_entity(), LuaEntity.destroy(), etc, methods took optional additional parameter `send_event` which if `true` (default: `false`) would generate appropriate events containing `raised = true` in their data tables.

[Optera]

I would make data.raised_by = "modname".
Sometimes it may be needed to treat events differently depending on what mod raised it.

[aubergine18]

I would make data.raised_by = "modname".
Sometimes it may be needed to treat events differently depending on what mod raised it.

Very good point! +1 And strings are truthy so in cases where the mod name doesn't matter it can still be treated as a boolean. For game raised events it would be `nil`, a falsey value.

[Rseding91]

Why should such a property exist? The game state and safety guarantees are identical if the core game fires an event or a mod fires an event - no special handling needs to be done by anyone listening.

[Mooncat]

In addition, it would be nice if the LuaSurface.create_entity(), LuaEntity.destroy(), etc, methods took optional additional parameter `send_event` which if `true` (default: `false`) would generate appropriate events

I second this. It will safe us plenty of time to find the appropriate event and provide it the expected parameters.
But I think more have to be done besides the "send_event" parameter in order to make it work.

The "player" parameter of LuaSurface.create_entity() is just about fast-replace so far. It has to be tweaked so that it can also be used for the "player_index" of the on_built_entity event.

And about LuaEntity.destroy(), I don't know which event should be raised. The entity is not killed, but simply vanished, so on_entity_died doesn't sound right. It is not mined, so it isn't on_player_mined_item.

In addition, I hope this can also apply to LuaSurface.set_tiles. Raising the appropriate event for it is the most tricky one to deal with so far. set_tiles accept an array of Tiles, while on_player_built_tile and on_player_mined_tile accept array of Positions. So I have to create 2 arrays. Hopefully this can be done in the game engine.


But back to the original topic, I have no idea why you need such thing either. If the event is raised correctly, shouldn't it have no difference no matter it is raised by the game or by any mod?
Unless... this is for debugging? To find the mod that mis-raised the event?

[aubergine18]

Why should such a property exist? The game state and safety guarantees are identical if the core game fires an event or a mod fires an event - no special handling needs to be done by anyone listening.

The idea was triggered by this discussion: viewtopic.php?p=216896#p216896

[Rseding91]

Why should such a property exist? The game state and safety guarantees are identical if the core game fires an event or a mod fires an event - no special handling needs to be done by anyone listening.

The idea was triggered by this discussion: viewtopic.php?p=216896#p216896

Sigh. I wish there were events for when entities get destroyed or created by mods so we don't have to check for validity all the frickin' time. I'll have a look into this. Thanks for letting me know.

I don't implement API changes for lazy mod developers. Checking "valid" IS THE CORRECT WAY TO WRITE A MOD.

[aubergine18]

There were two discussions going on in that thread. I agree that checking .valid (and in case of players, .connected) is vital and do that in my mods. @supercheese also points out the need for checking .valid, it's best practice, we know that. Even @mknejp agrees that he'll have to include the .valid tracking in his mod.

It was when he mentioned the assumptions made by mods about vanilla events that made me realise my mods also have encoded assumptions in them - if they get a on_entty_built event, they assume the player built it, that the player must be nearby (different event is fired if robot builds it), that the event table has all the expected fields, that the player must have either the corresponding item stack in their cursor or an empty slot in their quickbar or inventory (although on further thinking this is not necessarily true) and so on.

Without knowing whether an event is from vanilla game or mod generated, it just means I'll have to add far more extensive game state checking and tracking to my mods.

Example: When the first end of a bridge is placed in my mod, I show an "overlay" (just deactivated flying-text) over any deepwater tiles that a wood bridge can't cross. However, if another mod places that entity and fires the event, my mod still assumes the player did it (it has no way to know otherwise without checking player proximity to created entity) and renders the overlay.

Not a particularly good example, I know, but it's been a long day and best I could come up with before heading to bed.

Having an extra property in the table for raised events would act as an alert for my mod to do extra state checking, which could otherwise be skipped if I know it's a vanilla event.

[Optera]

I don't see passing the event raiser in events as something only for lazy modders.
Passing along what raised the event allows for selective event handling without making wrong assumptions or a lot of time consuming checks.

[mknejp]

Sigh. I wish there were events for when entities get destroyed or created by mods so we don't have to check for validity all the frickin' time. I'll have a look into this. Thanks for letting me know.

I don't implement API changes for lazy mod developers. Checking "valid" IS THE CORRECT WAY TO WRITE A MOD.

Emphasis on "all the frickin' time".

The current set of game events does not allow to reliably track the lifetime/validity of entities, therefore making it necessary to do extensive validation checking that should be possible to be circumvented if all relevant events are appropriately processed. If every single change in lifetime is preceded with an event without exception validation checking is not required if following the event protocol to the letter. It gives a certain implied guarantee about the state of entities. Unfortunately create_entity() and (most importantly) destroy() prevent this from being the case thus making excessive validation checking a unfortunate necessity. I'd rather implement numerous event handlers than negatively impacting performance with validity checks at every interaction with the game state.

[Rseding91]

Emphasis on "all the frickin' time".

The current set of game events does not allow to reliably track the lifetime/validity of entities, therefore making it necessary to do extensive validation checking that should be possible to be circumvented if all relevant events are appropriately processed. If every single change in lifetime is preceded with an event without exception validation checking is not required if following the event protocol to the letter. It gives a certain implied guarantee about the state of entities. Unfortunately create_entity() and (most importantly) destroy() prevent this from being the case thus making excessive validation checking a unfortunate necessity. I'd rather implement numerous event handlers than negatively impacting performance with validity checks at every interaction with the game state.

There's entity/item/other removal due to mod migration, game version migration, mod removal. There's also the core game itself removing entities when things like terrain are built over decoratives (they aren't killed, they're just destroyed) and so on.

Eventing for all of these things isn't possible and if the game was changed to make it possible it would waste a *TON* of CPU time for something that virtually everyone doesn't care about.

[mknejp]

There's entity/item/other removal due to mod migration, game version migration, mod removal.

There are events to handle these.

There's also the core game itself removing entities when things like terrain are built over decoratives (they aren't killed, they're just destroyed) and so on. Eventing for all of these things isn't possible and if the game was changed to make it possible it would waste a *TON* of CPU time for something that virtually everyone doesn't care about.

How many mods care about shrubbery being deleted?

Obviously it should only occur where it makes sense. But create_entity() and destroy() are functions that can completely screw up mods and the only prevention method is spamming .valid all around the place. This is not about being lazy, it's about correctness and performance. Excessive calls to .valid cannot be healthy as they imply a cache miss almost every time in places where it should not have to occur. Let the game only fire on_created/on_destroyed events in cases where mods call these functions since the game only removes stuff that, as you say, nobody cares about.

[Supercheese]

Why is checking .valid so hard?

[Klonan]

Why is checking .valid so hard?

Its not in the slightest... if you are doing anything to an entity... check if it is valid first
If you are doing something to an entity, the game will have to evaluate it anyway, so checking if its valid adds almost nothing to the calculation time


I even do it for forces just to be safe, check force.valid when i am doing something to a force, it doesn't really hurt

[mknejp]

Why is checking .valid so hard?

Its not in the slightest... if you are doing anything to an entity... check if it is valid first
If you are doing something to an entity, the game will have to evaluate it anyway, so checking if its valid adds almost nothing to the calculation time

Except it involves two table lookups only to get the metatable entry, followed by an array access to get the userdata, followed by Lua->C++ marshalling, involving at least one indirect call, error frame setup and parameter validation, potentially followd by a virtual call, followed by the actual checking logic, followed by returning to Lua which involves potential error recovery and parameter popping. I count 6+ dcache misses and 3+ icache misses, one of them (or three if a virtual call is also involved) being unpredictable making the prefetcher useless. I wouldn't call this "almost nothing". All of this then has to be repeated to do the actual call to the actual action hoping not all the dcache has been invalidated but you still pay for at least another 2+ dcache and 1+ icache miss before calling any code doing meaningful work.

The less Lua->C++ transitions there are the better.

Sure, it's not "hard" to do the validation. On the human factor you just have to remember doing it. On the technical factor the fewer calls you have to do to C++ the better, unless the C++ code can more efficiently do what you'd otherwise have to write in Lua to get the same result and even then it's a tradeoff that isn't always obvious because cache is a thing and the language transition has hidden costs to it.

If there are events that reliably cover all lifetime changes of "interesting" entities you implicitly know an entity is guaranteed valid until you receive an event telling you otherwise. Such a system leads to far less defensive checking and the associated overheads.

If there is one phrase that should strike terror in anyone concerned about performance it's "cache miss". It's the one single most critical impact on execution time on modern hardware.

[aubergine18]

Another discussion that ties in with full lifecycle event coverage: viewtopic.php?f=34&t=34952

Also, there are some specific cases where you don't need to check .valid, such as entities, players, etc., returned from an API call are at that precise moment valid - however, if you change them, or cache them local/global, then you'll still need to check .valid: viewtopic.php?p=217560#p217560