change report

================================================================================
  MOON-ENEAS MULTIPLAYER DESYNC FIX REPORT
  Files changed: camera.lua, events.lua, talk.lua, message.lua
  Additional fixes: debris mining crash + exploit, message.lua math.random
================================================================================

Hey, I looked through your mod's code and found what's causing the
desyncs, and also spotted the crash + exploit issue from the other
discussion thread. Nothing about gameplay was touched beyond what needed changing.

Here's a breakdown of what I found and changed, and why the original code
was breaking things.

TLDR:
- Don't use math.random(), use storage table to store predetermined rng values
  seeded by game.create_random_generator()
- Don't call script.on_nth_tick() during runtime, it's meant to be only called
  at top level of any script or called in on_init, on_load and on_configuration_changed,
  which are called only when the game is loaded, never during runtime.

- Changed how script.on_nth_tick is called in camera.lua, using storage to
  determine when to clean up.
- Added predetermined random generators for various situations inside the storage
  table, changed math.random calls to call those random generators instead.
- Fixed the full inventory crash and the infinite loot exploit by switching from
  on_pre_player_mined_item to on_player_mined_entity.


================================================================================
  FIRST, SOME CONTEXT ON HOW FACTORIO MULTIPLAYER ACTUALLY WORKS
================================================================================

This is worth understanding before diving into the fixes, because both issues
come from the same underlying misunderstanding of how Factorio handles joining
players.

Factorio multiplayer is NOT like most games where the server streams world state
to clients. Instead, every client runs the full simulation independently, tick
by tick. The server only sends player inputs (clicks, keypresses, that kind of
thing.) Because of this, every client must produce bit-for-bit identical game
state on every tick.

When someone joins a running game, Factorio sends them two things:
  1. The current save file (which includes the `storage` table and all entity
     state)
  2. A list of which event handlers the mod has registered

The joining client loads the save, re-registers all its handlers by running the
mod scripts fresh, and then both the server and the new client run forward from
that point in lockstep.

This means two things have to be true for a mod to be multiplayer-safe:

  RULE 1 - Event handler registrations must be identical on all clients.
  Any call to script.on_event(), script.on_nth_tick(), etc. must happen at load
  time, meaning at the top level of your scripts, or inside on_init / on_load /
  on_configuration_changed. A joining client re-runs those registrations from
  scratch. If the server registered a handler mid-game (inside a function that
  ran during gameplay), the joining client will never register that handler, and
  Factorio blocks the join with exactly the error you've been seeing: "mod event
  handlers are not identical."

  RULE 2 - Random numbers used in game logic must be deterministic.
  Lua's math.random() is seeded by the Lua VM at startup, usually from the
  system clock. That seed is different on every machine and is not saved. So
  every client that starts up (including someone joining mid-game) begins its
  math.random() sequence from a different point. Any call to math.random() in
  a game event handler will produce different results on different clients,
  which means they'll create different entities, place things at different
  positions, etc. The game states diverge silently, and eventually you get a
  desync crash.


================================================================================
  FILE 1: camera.lua
  Problem: script.on_nth_tick() being called inside a runtime function
================================================================================

--- WHAT CHANGED ---

Inside ShowStaticTransmission(), there was this block at the end:

    local cleanup_tick = game.tick + ((duration or 8) + 1) * 60
    script.on_nth_tick(61, function()
        if game.tick >= cleanup_tick then
            if static_entity and static_entity.valid then
                static_entity.destroy()
            end
        end
    end)

That's been replaced with:

    storage.static_transmission_cleanup = {
        cleanup_tick = game.tick + ((duration or 8) + 1) * 60,
        position = STATIC_POSITION,
    }

And a new permanent handler has been added at the top level of camera.lua
(outside any function, so it runs once when the mod loads):

    script.on_nth_tick(61, function()
        if not storage.static_transmission_cleanup then return end
        local pending = storage.static_transmission_cleanup
        if game.tick < pending.cleanup_tick then return end

        local nauvis = game.surfaces["nauvis"]
        if nauvis and nauvis.valid then
            local dummies = nauvis.find_entities_filtered{
                position = pending.position,
                radius = 5,
                name = "unit-05-dummy"
            }
            for _, entity in pairs(dummies) do
                entity.destroy()
            end
        end

        storage.static_transmission_cleanup = nil
    end)


--- WHY THE ORIGINAL CODE WAS BREAKING THINGS ---

ShowStaticTransmission() is called by on_research_finished() in talk.lua every
time a tech gets researched. That happens mid-game, at runtime, not at load
time.

Every time it ran, it called script.on_nth_tick(61, ...) with a new anonymous
function. In Factorio, calling script.on_nth_tick() at runtime doesn't add an
extra handler, it replaces whatever was registered for that interval with the
new one. So after each tech research, the handler registered for interval 61
changed to a new closure.

Now imagine someone tries to join the server after a tech has been researched.
They load the mod from scratch. They've never had a tech fire during their
session, so they've never called script.on_nth_tick(61, ...) mid-game. Their
handler for interval 61 is either empty or is whatever got registered at load
time. The server has a different, mid-game handler still active. Factorio
compares the two handler tables, finds they don't match, and blocks the join.

This is also why you mentioned the bug feels timing-dependent and gets worse
with more players. More techs researched means more chances for the handler to
be in a mid-game state when someone tries to join, and more players means techs
get researched faster.

The fix is straightforward: instead of registering a new handler each time a
transmission fires, we just write the cleanup information into `storage`. Since
`storage` is part of the save file, a joining player loads it automatically and
gets the correct cleanup tick. The one permanent handler registered at load time
(which every client always has) then picks it up on the next tick it fires.

One extra note: the original code stored `static_entity` directly in the closure
as a LuaEntity reference. That's fine for a closure, but LuaEntity references
can't go into `storage` either, Factorio will throw an error on save if you try. So
instead, we store the entity's position as a plain coordinate table and find it
again at cleanup time using find_entities_filtered. Since the dummy always spawns
at {100000, 100000}, this works reliably.


================================================================================
  FILE 2: events.lua
  Problem 1: math.random() used throughout all runtime event handlers
  Problem 2: Crash on full inventory + infinite loot exploit
================================================================================

--- WHAT CHANGED ---

Five deterministic random generators are now created in init_debris() and stored
in storage:

    storage.rng = storage.rng or {}
    storage.rng.debris    = storage.rng.debris    or game.create_random_generator()
    storage.rng.pollution = storage.rng.pollution or game.create_random_generator()
    storage.rng.loot      = storage.rng.loot      or game.create_random_generator()
    storage.rng.talk      = storage.rng.talk      or game.create_random_generator()
    storage.rng.spawn     = storage.rng.spawn     or game.create_random_generator()

Every math.random() call in a runtime function has been replaced with the
appropriate generator. Full list of replacements:

  FUNCTION: find_valid_spawn_location()
    math.random(-spawn_radius, spawn_radius)   ->  storage.rng.debris(min, max)
    (x2, for x and y coordinates)

  FUNCTION: select_random_debris()
    math.random() * total_weight               ->  storage.rng.debris() * total_weight

  FUNCTION: scatter_fragments()
    math.random() * 2 * math.pi               ->  storage.rng.debris() * 2 * math.pi
    math.random() * scatter_radius            ->  storage.rng.debris() * scatter_radius

  FUNCTION: execute_debris_crash()
    math.random(i)  [surface shuffle]         ->  storage.rng.debris(i)
    math.random(min_fragments, max_fragments) ->  storage.rng.debris(min, max)

  FUNCTION: check_timed_events()
    math.random(min_interval, max_interval)   ->  storage.rng.debris(min, max)
    math.random(5*60*60, 15*60*60)            ->  storage.rng.debris(min, max)
    (these two schedule the next debris crash after a successful or failed attempt)

  FUNCTION: execute_gas_venting()
    math.random(vent_min, vent_max)           ->  storage.rng.pollution(min, max)

  FUNCTION: create_scaled_rewards()
    math.random(amounts.min, amounts.max)     ->  storage.rng.pollution(min, max)

  FUNCTION: find_supply_drop_position()
    math.random() * 2 * math.pi              ->  storage.rng.debris() * 2 * math.pi
    math.random() * SUPPLY_DROP_SPAWN_RADIUS ->  storage.rng.debris() * radius
    (note: even though the comment says this function is "old / not used for
    Eneas", it is still actively called from create_supply_drop_for_player()
    which gets triggered on tech research, so it wasn't dead code)

  FUNCTION: on_debris_mined()
    math.random()                             ->  storage.rng.loot()
    math.random(#bonus_loot_table)            ->  storage.rng.loot(N)
    math.random(min_count, max_count)         ->  storage.rng.loot(min, max)


--- WHY THE ORIGINAL CODE WAS BREAKING THINGS ---

As explained above, math.random() is seeded locally on each machine at startup.
Two clients running the same game will have completely different math.random()
sequences from the very first call.

The practical effect: when on_debris_mined() fires and calls math.random() to
decide if a bonus loot item drops, the server might roll 0.03 (yes, drop a
productivity module) while a joining client rolls 0.74 (no drop). The server
creates the item entity. The client doesn't. Their entity counts are now
different. That's a desync.

The same thing happens in scatter_fragments() where math.random() picks the x/y
coordinates for each debris piece. Different coordinates on server vs. client
means different entity positions. Different positions means different game state.

What makes this really tricky to diagnose is that math.random() desyncs are
silent and cumulative. Each divergent call pushes the clients a little further
apart. By the time Factorio actually detects and reports the desync, you're often
many ticks past the original cause, which makes it nearly impossible to find
without specifically auditing every math.random() call.

The fix is game.create_random_generator(). This is Factorio's own RNG, seeded
from the map seed which is identical on all clients. Its internal state gets
stored in `storage`, so it's included in the save file. When someone joins,
they load the generator in exactly the state the server has it, and from that
point all clients produce the same sequence.

The `or` guards on each generator (e.g. `storage.rng.debris = storage.rng.debris
or ...`) are important. init_debris() gets called again by on_configuration_
changed when the mod is updated. Without those guards, updating the mod in an
existing game would reset all the generators and introduce new divergence.

I split it into five separate generators (debris, pollution, loot, talk, spawn)
rather than one shared one. If they all shared a single generator, then the
debris system firing a few extra times (e.g. because players cleared debris
faster than usual) would shift the sequence for pollution and loot rolls too.
Keeping them separate means each system's randomness is independent and
predictable.



================================================================================
  FILE 2: events.lua (continued)
  Problem 2: Crash on full inventory + infinite loot exploit
================================================================================

--- WHAT CHANGED ---

Two things in the event registration block at the bottom of events.lua.

First, the spill_item_stack call inside on_debris_mined() was updated to match
the Factorio 2.0 API. The old call passed 5 separate arguments, which is the
Factorio 1.x syntax. Factorio 2.0 changed it to take a single table:

    BEFORE:
        player.surface.spill_item_stack(
            player.position,
            {name = item_name, count = count - inserted},
            true,
            player.force,
            false
        )

    AFTER:
        player.surface.spill_item_stack{
            position = player.position,
            stack = {name = item_name, count = count - inserted}
        }

Second, the event the handler listens to was changed:

    BEFORE:
        script.on_event(defines.events.on_pre_player_mined_item, on_debris_mined)

    AFTER:
        script.on_event(defines.events.on_player_mined_entity, on_debris_mined)


--- WHY THE ORIGINAL CODE WAS BREAKING THINGS ---

The crash is straightforward. spill_item_stack changed its signature in Factorio
2.0 and the old multi-argument call just doesn't work anymore. That's exactly
what the error in the bug report says: "Expected 1 argument but 5 were given."

The exploit is more subtle and comes from which event was being used.
on_pre_player_mined_item fires the moment the player starts the mining animation,
before the game knows whether mining will actually succeed. If the player's
inventory is full, Factorio cancels the mining and the debris stays in the world.
But the event already fired, so the loot roll already happened and any bonus
items were already handed out.

That means a player can stand next to a debris piece with a full inventory and
spam-click it forever. The debris never gets consumed, but every click has a
chance to roll bonus loot and drop tier 3 modules on the ground. Infinite free
modules at no cost.

Switching to on_player_mined_entity closes this completely. That event only fires
after mining has successfully finished and the entity has been removed from the
world. If the inventory was full and mining was cancelled, the event never fires,
the loot roll never happens.

The robot handler was already using on_robot_mined_entity (the post-mining
equivalent for robots), so that one didn't need changing.


================================================================================
  FILE 3: talk.lua
  Problem: math.random() used to pick unit-05 dialogue lines
================================================================================

--- WHAT CHANGED ---

One line in trigger_message():

    BEFORE:
        local msg = unit05_messages[math.random(#unit05_messages)]

    AFTER:
        local msg = unit05_messages[storage.rng.talk(#unit05_messages)]

The storage.rng.talk generator is created in init_debris() in events.lua,
it's the fourth one in the init block shown above.


--- WHY THE ORIGINAL CODE WAS BREAKING THINGS ---

trigger_message() runs inside a script.on_nth_tick(3600, ...) handler, once
every in-game hour, for any player standing within 10 tiles of a unit-05
entity. That handler runs in the synchronized game simulation on every client.

The tricky part here is that rendering.draw_text is called with
`players = {player}`, so the text bubble only appears for that specific player.
That part is fine. But math.random() gets called before that, and it runs on
every client regardless. So even though you never see any visual difference,
every client is independently advancing its math.random() sequence at different
rates depending on how many players are near unit-05 on each client's version of
the game.

This shifts the math.random() sequence differently on every client, and then
the next time something like debris spawn fires and calls math.random() for
coordinates, everyone gets different values. It's one of those bugs that is
totally invisible in single player and very hard to connect to the actual
desync crash in multiplayer.

Same fix as events.lua, use storage.rng.talk which is synchronized and
deterministic. I kept it as a separate generator from the others so that how
often players chat with unit-05 doesn't interfere with debris or loot rolls.


================================================================================
  FILE 4: message.lua
  Problem: math.random() in spidertron spawn and servofish capsule handler
================================================================================

--- WHAT CHANGED ---

Two math.random() calls replaced in message.lua.

First, find_water_spawn_position() which picks a random location on Eneas water
to spawn the unit-07 spidertron when lurker-tech is researched:

    BEFORE:
        local angle = math.random() * 2 * math.pi
        local distance = math.random() * radius

    AFTER:
        local angle = storage.rng.spawn() * 2 * math.pi
        local distance = storage.rng.spawn() * radius

Second, the servofish capsule handler which picks a random unit-05 voiceline
when a player uses a servofish item:

    BEFORE:
        local message = voicelines.servofish[math.random(#voicelines.servofish)]

    AFTER:
        local message = voicelines.servofish[storage.rng.talk(#voicelines.servofish)]

The storage.rng.spawn generator needs to be added to init_debris() in events.lua
alongside the others. storage.rng.talk is already there from the talk.lua fix.


--- WHY THE ORIGINAL CODE WAS BREAKING THINGS ---

find_water_spawn_position() runs inside spawn_spidertron_for_force(), which is
called from on_research_finished() when the lurker-tech is researched. It uses
math.random() to pick coordinates for where the spidertron spawns. Since this
creates an entity in the world, different clients landing on different coordinates
means different entity positions, which is a desync.

The servofish handler is the same situation as the unit-05 dialogue in talk.lua.
The voiceline only appears to the player who used the capsule, but math.random()
still runs in the simulation on every client, silently diverging their RNG
sequences. Same fix, same reasoning, so it reuses storage.rng.talk.


================================================================================
  SUMMARY TABLE
================================================================================

  File         | Location                      | Old code              | New code

  camera.lua   | ShowStaticTransmission()      | script.on_nth_tick()  | storage.static_transmission_cleanup = {...}
               |                               | called at runtime     |
               | (new, top-level)              | (missing)             | script.on_nth_tick(61, ...) at load time

  events.lua   | init_debris()                 | (missing)             | storage.rng.* = game.create_random_generator()
               | find_valid_spawn_location()   | math.random(min, max) | storage.rng.debris(min, max)  [x2]
               | select_random_debris()        | math.random()         | storage.rng.debris()
               | scatter_fragments()           | math.random()         | storage.rng.debris()          [x2]
               | execute_debris_crash()        | math.random(i)        | storage.rng.debris(i)
               |                               | math.random(min, max) | storage.rng.debris(min, max)
               | check_timed_events()          | math.random(min, max) | storage.rng.debris(min, max)  [x2]
               | execute_gas_venting()         | math.random(min, max) | storage.rng.pollution(min, max)
               | create_scaled_rewards()       | math.random(min, max) | storage.rng.pollution(min, max)
               | find_supply_drop_position()   | math.random()         | storage.rng.debris()          [x2]
               | on_debris_mined()             | math.random()         | storage.rng.loot()
               |                               | math.random(N)        | storage.rng.loot(N)
               |                               | math.random(min, max) | storage.rng.loot(min, max)

  talk.lua     | trigger_message()             | math.random(N)        | storage.rng.talk(N)

  events.lua   | on_debris_mined()             | spill_item_stack(     | spill_item_stack{
               |                               |   5 args, old API)    |   position=..., stack=... }
               | event registration            | on_pre_player_        | on_player_mined_entity
               |                               |   mined_item          |

  message.lua  | find_water_spawn_position()   | math.random()         | storage.rng.spawn()           [x2]
               | on_player_used_capsule        | math.random(N)        | storage.rng.talk(N)