Re-opened: breaking change/fix: script.raise_event rework

Place to post guides, observations, things related to modding that are not mods themselves.
User avatar
Optera
Smart Inserter
Smart Inserter
Posts: 2916
Joined: Sat Jun 11, 2016 6:41 am
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by Optera »

You certainly are correct about comments like "your code is a pile of trash", those are pointless trash worth less than any code could ever be.
However any grown up should know how to handle constructive criticism like "on_built_entity will break with this change, use script_raised_built instead."
That said, there are the vocal entitled snowflakes who cant take criticism. Instead of tip-toeing around those just call things as they are and ignore the fallout from them.

Sorry for the rant, back to the juicy data.
Most mods in the list can be grouped in either of these 3 categories:
  1. passed down mods, some forked several times, with the original author long gone.
    Forcing those to finally update properly instead of just raising version in config.ini sounds like a good thing to me.
  2. the old ones either too complex to refactor or lacking script_raised_* equivalents.
    Those authors usually will make it work, hopefully by getting the api extended instead of having to find new hacks.
  3. small new mods
    Those authors probably saw bad practice used in the former two groups and copied it. Breaking compatibility with outdated, now bad, practices instead of deprecating makes finding good practice code to lean on easier.

robot256
Filter Inserter
Filter Inserter
Posts: 596
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by robot256 »

Most of the ones using script_raised_ don't do anything else, so it does indeed look like it's a before/after split and just old mods not fully updated to raise the new events.
Not sure this is what you mean, but in one case I call raise_event(script_raised_built) so that I can modify an entity before alerting other mods to its existence.

Bilka
Factorio Staff
Factorio Staff
Posts: 3128
Joined: Sat Aug 13, 2016 9:20 am
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by Bilka »

The changes are coming
Next week, with the mod breaking gui changes in 0.18.27, we will also implement the script.raise_event rework discussed here in this thread.
Summary
script.raise_event will be restricted to raising events generated with script.generate_event_name and raising the following built-in events:
  • on_console_chat
  • on_player_crafted_item
  • on_player_fast_transferred
  • on_biter_base_built
  • on_market_item_purchased
  • script_raised_built
  • script_raised_destroy
  • script_raised_revive
  • script_raised_set_tiles
No other built-in events will be raisable directly.

All the raisable built-in events will have the data that you pass to them validated so that mandatory fields must be present and fields must have the correct types. Failing to provide correct and complete event data will result in an error for the mod raising the event.
Extra data that is provided in the event data table e.g. non_existant_event_field = "foo" is not passed along with game events, however the 4 script_raised_* events will pass it along.

Event filtering now also works for events raised by mods, this was the major point of these changes.

Already released in 0.18.26 are the following new events and extra ways to automatically raise events:
  • Added script_raised_set_tiles.
  • Added by_player to LuaEntity::copy_settings()
  • Added by_player to LuaEquipmentGrid::take, take_all, clear, and put.
More information
Replacements for raise event calls that no longer work
Thanks to JanSharp, we have a list of raise_event calls that mods use. Based on that, I am providing you a list of how to replace your (soon to be) broken raise event calls:

Direct replacement:
  • on_entity_spawned - use script_raised_built
  • on_entity_died - use script_raised_destroy
  • on_player_built_tile - use script_raised_set_tiles/script_raised_revive
  • on_robot_built_entity - use script_raised_built/script_raised_revive
  • on_robot_built_tile - use script_raised_built/script_raised_revive
  • on_robot_mined_tile - use script_raised_destroy
  • on_robot_pre_mined - use script_raised_destroy
Game raises event automatically:
  • on_player_armor_inventory_changed - the game raises this automatically
  • on_player_cursor_stack_changed - the game raises this automatically
  • on_player_driving_changed_state - the game raises this automatically
Function raises event automatically:
  • on_entity_settings_pasted - use by_player in LuaEntity::copy_settings() which raises the event automatically
  • on_player_placed_equipment - use by_player in LuaEquipmentGrid::take, take_all, clear, and put which raises the event automatically
  • on_player_rotated_entity - use LuaPlayer.rotate() which raises the event automatically
  • on_pre_player_mined_item - use LuaPlayer.mine_entity() which raises the event automatically
  • on_player_respawned - set ticks_to_respawn = 0 for respawning which raises the event automatically
  • on_resource_depleted - use LuaEntity.deplete() which raises the event automatically
Function raises event automatically, alternative script_raised_*:
  • on_player_mined_item - use LuaPlayer.mine_entity() which raises the event automatically, alternatively use script_raised_destroy
  • on_built_entity - use LuaPlayer.build_from_cursor() which raises the event automatically, alternatively use script_raised_built/script_raised_revive
  • on_entity_cloned - the clone function raises the event automatically, alternatively use script_raised_built
Special case:
  • on_gui_click - use remote interfaces to interact with other mods (mod was using this to "click" gui elements)
Some notes on raisability removal reasons
None of the on_robot_* events that were being raised by mods had valid construction robots being provided. For that reason, these can replaced with the generic script_raised_* events without losing information.

Originally on_built_entity was planned to be raisable due to being whopping 40 out of the 100 raised game events (does not include script_raised_*) and we did not want to break all those mods. However, I went through all 40 places where the event was being used, and it turned out that only 5 of those uses were giving all mandatory fields (and 3 of those places were just copies of the other places). This means that all 35 other raise_event(on_built_entity,..) calls would have broken anyways due to the new validation. So, we decided to remove the ability to raise the event directly, since it results in the same amount of broken mods while also forcing mods to raise the correct events.

Extra features
The following functions are being added in 0.18.27, they allow you to raise the built-in events without using script.raise_event, so you can reserve that for custom mod events. You don't have to use these functions, script.raise_event will continue to work for the events listed in the summary.
  • script.raise_console_chat
  • script.raise_player_crafted_item
  • script.raise_player_fast_transferred
  • script.raise_biter_base_built
  • script.raise_market_item_purchased
  • script.raise_script_built
  • script.raise_script_destroy
  • script.raise_script_revive
  • script.raise_script_set_tiles
The following three events can now be filtered based on the entity passed to them:
  • script_raised_built
  • script_raised_destroy
  • script_raised_revive
My tip
You can already replace all your soon to be broken raise_event calls right now since all listed fixes already work (if you dont want to use the new raise_* functions). If you do that, make sure to double-check that you are raising the raisable events with the correct fields. If you do these fixes now, you wont have to do them at the same time as the GUI fixes :)

Feedback
We continue to listen to feedback on this and will adjust things if we feel it's beneficial, so please feel free to discuss these changes.

Disclaimer
There is always the possibility that these changes are delayed/the changes change. I will update this post if that is the case.
I'm an admin over at https://wiki.factorio.com. Feel free to contact me if there's anything wrong (or right) with it.

User avatar
Therax
Filter Inserter
Filter Inserter
Posts: 470
Joined: Sun May 21, 2017 6:28 pm
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by Therax »

I recently received a bug report where another mod is using create_blueprint(), and this was bypassing my mod's on_player_setup_blueprint handling. My proposed fix was for the other mod to raise the event manually, but since this approach will no longer be accessible, it makes it more important that either an event be raised automatically on function call to LuaItemStack::create_blueprint (viewtopic.php?f=28&t=85012) or some other approach made available (script_raised_setup_blueprint?).
Miniloader — UPS-friendly 1x1 loaders
Bulk Rail Loaders — Rapid train loading and unloading
Beltlayer & Pipelayer — Route items and fluids freely underground

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

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by Rseding91 »

Therax wrote:
Fri May 22, 2020 8:05 pm
I recently received a bug report where another mod is using create_blueprint(), and this was bypassing my mod's on_player_setup_blueprint handling. My proposed fix was for the other mod to raise the event manually, but since this approach will no longer be accessible, it makes it more important that either an event be raised automatically on function call to LuaItemStack::create_blueprint (viewtopic.php?f=28&t=85012) or some other approach made available (script_raised_setup_blueprint?).
There are multiple ways to create or edit a blueprint through script that don't have any event logic associated. Adding it for one and not the rest doesn't make sense to me. Additionally I have no plans to add events for all the ways it can be done so there's no point in adding it for any of them.

This kind of this is all over the mod API: mods doing things virtually never notify other mods they did it - and that's just how the entire API and game is built.
If you want to get ahold of me I'm almost always on Discord.

User avatar
Therax
Filter Inserter
Filter Inserter
Posts: 470
Joined: Sun May 21, 2017 6:28 pm
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by Therax »

Rseding91 wrote:
Sat May 23, 2020 7:08 am
There are multiple ways to create or edit a blueprint through script that don't have any event logic associated. Adding it for one and not the rest doesn't make sense to me. Additionally I have no plans to add events for all the ways it can be done so there's no point in adding it for any of them.
There are many ways to programmatically create and edit a blueprint through script, but only one that directly maps to the normal player action: capturing an area of a surface with a blueprinting tool. In the same way that there are many ways to destroy an entity and put the corresponding item into the player's inventory, but only one that executes the whole mining action start-to-finish, raising the appropriate sequence of events: LuaControl::mine_entity().

In any case, I only posted in this thread to call attention to a use case that might involve raising a game event not covered, although it seems no mods currently do so.
Miniloader — UPS-friendly 1x1 loaders
Bulk Rail Loaders — Rapid train loading and unloading
Beltlayer & Pipelayer — Route items and fluids freely underground

robot256
Filter Inserter
Filter Inserter
Posts: 596
Joined: Sun Mar 17, 2019 1:52 am
Contact:

Re: Re-opened: breaking change/fix: script.raise_event rework

Post by robot256 »

Bilka wrote:
Fri May 22, 2020 5:53 pm

Already released in 0.18.26 are the following new events and extra ways to automatically raise events:
  • Added by_player to LuaEquipmentGrid::take, take_all, clear, and put.
Function raises event automatically:
  • on_player_placed_equipment - use by_player in LuaEquipmentGrid::take, take_all, clear, and put which raises the event automatically
What is the best solution if a mod places equipment but no player is directly responsible, or no player reference is available, but it still needs to alert other mods that equipment was added to a grid? Previously I abused on_player_placed_equipment by using the entity's last_user, and occasionally raising it with player_index=nil. It seems that an official script_placed_equipment event might be preferable to every mod adding a custom event to handle this circumstance.

EDIT: Actually I did not ever set player_index=nil. But since I save last_user.index, there is a possibility that when I go to insert the equipment that player will have been removed from the game, and I won't be able to convert it to a LuaPlayer. So the event might not be called in some cases where it was before.

Post Reply

Return to “Modding discussion”