Core/LuaLib: `util.event_handler` module odd behaviour

[trail stone]

TL; DR

If you use event_handler across your project with different import names (see below), the one will overwrites events defined in the other.

Case 1

This results in a error (as expected)

require('event_handler').add_lib(module)
require('event_handler').add_lib(module)

Case 2

This also results in a error (as expected)

require('__core__.lualib.event_handler').add_lib(module)
require('__core__.lualib.event_handler').add_lib(module)

Case 3

This does not result in error (not expected!)

require('__core__.lualib.event_handler').add_lib(module)
require('event_handler').add_lib(module)

[wispy gate]

That is simply because they're different event handlers

[granite hemlock]

Files are stored via require, referenced by path

[wispy gate]

Only the second even handler will work :)

[granite hemlock]

The code executing twice is an "error" you say, but what is the error?

[trail stone]

So, even though they result in the same file, Factorio's Lua engine can't handle that they are the same.

[granite hemlock]

Is it an explicit "you added your events twice, ya donkey" or what

[trail stone]

You can't register the same module twice.

[granite hemlock]

Copy and paste the error.

[trail stone]

The problem is the case 3 wipes out the existing events; it's a shorter version of a problem I was debugging

[granite hemlock]

That's not a problem that's a feature.

[trail stone]

Yes, exactly.

[wispy gate]

That is something that is not at fault of the event handler
That is your fault for using different strings to require the event handler

[trail stone]

The problem is if you combine different import styles within your mod (or any files imported from other mods) that could result in completely broken imports event handlers.

[granite hemlock]

There are two situations going on, one is calling the thing on the same "module" twice results in an error (which is probably a good thing, since it is very unlikely you wanted that)

The second situation is overriding event handlers because you loaded the event handler twice.

[trail stone]

The 1st one is known, expected behaviour.

[trail stone]

That's the problem I am describing, yes

[wispy gate]

I guess we could add an api request to add the singleton header (check what it's required by and require by the 'correct' path if it's not that)
But in general it's your fault if you're inconsistent

[granite hemlock]

There's probably been work around it, I've seen path and require resolutions being discussed

[wispy gate]

And yes. The error is an explicit error written into event_handler
I've looked at it plenty-a-time to understand how exactly it worked

[granite hemlock]

Thank you. There's a language error vs the event handler [library] going "wtf are you doing, please don't"

[wispy gate]

Yes I'm describing it to Honktown

[trail stone]

ah, sorry

[granite hemlock]

https://www.lua.org/pil/8.1.html

The other main job of require is to avoid loading the same file twice. For that purpose, it keeps a table with the names of all loaded files. If a required file is already in the table, require simply returns. The table keeps the virtual names of the loaded files, not their real names. Therefore, if you load the same file with two different virtual names, it will be loaded twice.

It's not a bug it's a feature which has nothing to do with event handler

[trail stone]

oh.

+1 point on my list of Lua broken/insane/horrible "features"

[wispy gate]

There are legitimize uses for it
Not many, but there are

[granite hemlock]

Yeah, I mean, it's weird they do it that way, but in theory, you can't "perfectly" predict whether a required file should be cached or loaded repeatedly.

[trail stone]

That would be #16

[wispy gate]

But there is also the way of making a file a singleton:

if (...) ~= "cannonical/require/path" then
  return require("cannonical/require/path")
end

[granite hemlock]

Given different names, or hell, the same name. I hope which never happens, anyways.

I mean, if anything, there may be a difference in how you're modelling the code vs how it is

[trail stone]

Just have two maps, one for import name, one for real name.

[granite hemlock]

Factorio is very heavily "libraries and functions on data". Store the library in a variable after requiring it and use it from there.

[trail stone]

The event_handler lib is the exception; it's purpose is to allow different modules register their own events such that all of them are called.

[wispy gate]

Yeah. Make an api request to add the singleton header to it

[granite hemlock]

Calling a library and immediately using it is a little... hrm. It makes sense in a stricter language but Lua only really uses it on require.

Tables can mutate on every access, which makes caching A and B which look like the same line can't be guaranteed to work.

Again, the error had nothing to do with the require, it was event handler telling you not to call it twice on the same thing.

[wispy gate]

Which, again Hares understands

Their entire issue is that it wasn't erroring

[lilac dune]

luals is supposed to warn you about requiring teh same file multiple different ways

[wispy gate]

Because they had two different event handlers (because the path to get to it was different)

[lilac dune]

but sometimes it trips on our plugin to help with mod prefixes

[granite hemlock]

Yeah, which is a discrepancy in the model, no criticism there. There's learning happening, I think.

[lilac dune]

afaik the given exampel it woudl ahve warned on though, at least last time i was testing that

[trail stone]

pseudo-code of singleton require in Python

ModuleName = NewType('ModuleName', str)
loaded_modules: Dict[ModuleName, Module] = dict()
loaded_files: Dict[Path, Module] = dict()

def resolve_name(module_name: ModuleName) -> Iterable[Path]:
    ...
def require(module_name: ModuleName) -> Module:
    if (module_name in loaded_modules):
        return loaded_modules[module_name]
    for path in resolve_name(module_name):
        if (path in loaded_files):
            loaded_modules[module_name] = loaded_files[path]
            return loaded_files[path]
        elif (path.is_file()):
            loaded_files[path] = load_module(path)
            loaded_modules[module_name] = loaded_files[path]
            return loaded_files[path]
    raise ImportError(f"Module {module_name!r} not found")

[wispy gate]

Just going to pull this right down here about making a file a singleton in lua

[lilac dune]

doesn't really help here for event_handler though

[wispy gate]

My stance is to just get them to make an api request to add it to the event handler

There really shouldn't be multiple anyways as they use script to register the on_init handler and multiple would absolutely conflict with each other

[trail stone]

No such warnings found in neither VSCode Problems, stdout/stderr, nor logs

[wispy gate]

Works for me

[trail stone]

Maybe the problem is because I am importing it in different files.

[lilac dune]

should still warn as long as it sees both at once

[trail stone]

That's the whole point why I started this thread.

[wispy gate]

Took mine a second, but I've got a lot in my workspace

[trail stone]

Ah. It doesn't understand that these are the same file :D

[granite hemlock]

I never used the core event handler, but scope pollution passes through requires

[lilac dune]

ah, then you probably dont' have fmtk installed properly

[granite hemlock]

control.lua could require and store it in a variable, and use that instead through all the other files

vs calling require directly

[trail stone]

Won't work for me; I am using imports from other mods

[granite hemlock]

Hijacking script is totally possible. Didn't this come up recently?

[trail stone]

Yes, that's the actually the same project.

[granite hemlock]

If you really want to, copy and paste the event handler into your mod 🤷. base/core code is "free as in beer and freedom but only for Factorio or whatever"

[trail stone]

These are my settings

[lilac dune]

that will only make things worse

[trail stone]

that will only make things worse.

[wispy gate]

Run check config to get more useful information

[trail stone]

And I can't add a hook to port libraries from event_handler to __core__.lualib.event_handler (or vice versa) because it's made local.

[lilac dune]

well, you can put anything you want in package.loaded

[granite hemlock]

I'm not involved in that part

[lilac dune]

preload it once, put it in every possible name in package.loaded

[trail stone]

Is package.loaded some kind of reserved name/API, or should I do it by myself?

[lilac dune]

it's just a table

https://www.lua.org/manual/5.2/manual.html#pdf-require but .loaded is the only one that is still real for us

(also https://lua-api.factorio.com/latest/auxiliary/libraries.html#•-require() )

[trail stone]

Done.

It worked flawlessly.

Thanks, @lilac dune

[lilac dune]

two pings for that? seriously?

[trail stone]

-# Oops, intended to do only once.

[lilac dune]

zero woudl be the correct number

since there is nothing actionable