EarendelDevelopers / factorio-mods

This is a public repository for tracking issues with Earendel's factorio mods.
19 stars 3 forks source link

Custom event for cargo rocket launch only triggers once per silo #171

Closed robot256 closed 2 years ago

robot256 commented 2 years ago

Copying here from the Discord so it doesn't get lost. Also reference my Not A Bug report on the forums.

Problem:
The custom on_cargo_rocket_launched_event triggers only on the first time a given silo is launched. Subsequent launches do not trigger the event.

Root Cause:
Invalid entity references are contained within the data={struct=struct} table passed to LuaBootstrap::raise_event(), causing the event to be silently ignored. (See linked forum post.)

The most important invalid entity is a reference to one of the landing animation elements that is deleted during the landing sequence, but the reference is not erased from the global table for the launchpad that created it. This affects all versions of Space Exploration.

Additionally, on very old saves, there can be two additional invalid entities. <launchpad_struct>.settings and <landingpad_struct>.settings are both invalid in the game that I migrated from Factorio 1.0 (started December 2019).

I found that ensuring all three of struct.settings, struct.launching_to_destination.landing_pad.settings and struct.rocket_entity are set to nil before raising the event makes everything work correctly.

robot256 commented 2 years ago

There could still be performance implications from sending the entire Zone tree with the event. (It includes, for example, the list of all occupied chunks on the source, destination, and their parent surfaces, which can be quite long.)

InappropriatePenguin commented 2 years ago

That's true. We'll likely replace the current struct that's provided by this event with a much leaner, shallower data structure. Zone references will be replaced with zone id's and some other properties will likely be withheld.

I did notice, after fixing the rocket_entity and settings issues, that the event still doesn't get consistently raised in my old saves, in contrast to in new games, where it works as expected. This is likely happening because, deep in the current struct, there can still be references to other invalid entities. I'm suspecting map markers inside of zone tables to be the culprit, since they can be destroyed/invalidated by the player.

What do you plan to use the event for?

robot256 commented 2 years ago

I'm trying to make a launch history GUI like the Train Log mod. What I'm planning to use so far is:

It would also be nice to have some extra information that I'm not sure is available right now:

InappropriatePenguin commented 2 years ago

The first list is mostly fine. Launchpad contents are a bit iffy, since that can be a big table to have to send each time. As for the nice-to-haves:

Earendel was contemplating adding a couple more cargo rocket related events, and potentially changing the way this group of events is raised in the first place. He's a bit busy with other SE-stuff at the moment though, so it might be a while before this is fully taken care of.

robot256 commented 2 years ago

Okay, sounds good. I could work with more events (launch, land, crash?). The contents table is at most 500 keys and integers, certainly no worse than the multiple zone tables being sent now. But I suppose if sending the LuaInventory handle is reliable I can call get_contents() on that every time.

I'm still working on my gui formatting with the current structure, but won'tt release it until the event at least fires every time.

InappropriatePenguin commented 2 years ago

Earendel may have been thinking about events for the different rocket launch phases (launch, surface transition, landing), but we didn't really hash it out. It will depend on whether we want that interface to allow other mods to influence the cargo rocket journey in some way (like forcing a crash under certain circumstances, say), vs. just passively being notified of a launch/landing.

We'll have to sort all that out before really thinking about the event table contents.

robot256 commented 2 years ago

Okay, sounds good. If you get a chance in a release before then to fix the event just for new savegames, by erasing struct.rocket_entity, that would be helpful.

robot256 commented 2 years ago

FYI, the temporary code I've been testing my mod with is

    local event_data = {
      container = struct.container,
      unit_number = struct.unit_number,
      zone = {index=struct.zone.index, name=struct.zone.name},
      launched_contents = struct.launched_contents,
      destination = {zone={index=struct.destination.zone.index, name=struct.destination.zone.name}},
      launching_to_destination = {
        landing_pad_name = struct.launching_to_destination.landing_pad_name,
        position = struct.launching_to_destination.position,
        landing_pad = struct.launching_to_destination.landing_pad and {container = struct.launching_to_destination.landing_pad.container},
      }
    }

    script.raise_event(Launchpad.on_cargo_rocket_launched_event, {struct = event_data})