I have an assembler with Speed modules. It is currently set to be upgraded to Efficiency. I right click drag a Productivity upgrade over it, causes the following error:

And probably the same bug, just doesn't crash:

I have an assembler 2 with Speed modules. It is currently set to be upgraded to Productivity. I right click drag the Productivity upgrade over it. It now requests 3 productivity modules
.

And since it is in the same piece of code, any chance to change this line (currently 176)

script.raise_event(on_module_inserted, {modules = {[module] = 1}, player = player, entity = entity})

to

script.raise_event(on_module_inserted, {modules = requests, player = player, entity = entity})

That, or also create a on_module_removed event (might make more sense).

Right now I can check in the on_module_inserted event the current requests in the entity, and the new requests in the event (since it is effectively a on_pre_module_inserted with the requests being set after the event is raised).
However this fails when using right clicks to only change one module, as only the new module is passed through. I'd have to reverse engineer the logic to see which one was removed.

Huge appreciation for the great code and raising the on_module_inserted event and raise build/destroy with the item request proxies!
I'm improving the Ghost Scanner mod to use events instead of scanning the surfaces at certain intervals, and MIS is essential when it comes to modules with the events it is throwing.

Thanks for the bug report - that seems like a fairly fundamental bit of functionality so it is strange that it is only being reported now... I guess most people don't use the right-click-drag. Or it is a bit more of a specific situation that causes the crash/bug.

And about the remote interface: I added it for better compatibility with Ghost Warnings, but I can certainly add a on_module_removed event. Or perhaps I'll rename them to on_module_request_created and on_module_request_removed as that seems more accurate.

since it is effectively a on_pre_module_inserted with the requests being set after the event is raised

Yes I suppose it is. I think this was so that in Ghost Warnings I can ignore the subsequent script_raised_built raised when the item-request-proxy is created if and only if it was created by MIS. If it was created by MI/MIE then I still want to use the item-request-proxy to give a best-effort warning (I don't get a player so I have to warn every player on the force, and it won't be raised if MI/MIE is only updating an existing item-request-proxy - I'm not sure they ever do though, that might just be MIS).

I don't think it is that strange. It only happens if there are modules marked for upgrade. Upgrading modules with this mod drops the old ones on the ground and marks them for deconstruction. Where vanilla marks them for upgrade immediately.

So this can only happen if someone first marks them for upgrade with the vanilla planner, and then uses the mod to adjust only one of them to something else. Indeed the right click functionality will already rarely be used, but having to have used an alternative planner first makes it extremely rare.

I just found out because I was rigorously testing all edge cases for my mod :P

The raising built/destroyed for the item-request-proxy I love as well. I don't understand why the base game doesn't raise those when the player or bots insert the modules. I listen to those events for all non ghost entities.

Since they are not raised for ghosts, I depend on on_module_inserted for those. So again, a really specific edge case where you have a ghost entity with ghost modules and you want to change one module in that ghost entity.