enums break type checking in 1.1.43.

[ahicks92]

I've confirmed amongst a bunch of people that this is 1.1.43, and it matches some of the changelog as well. These two lines of code should type check:

local x = defines.direction.east
x = defines.direction.west

But don't because the declaration file uses @as to get nameable per-variant types, convincing LuaLS that they aren't compatible. Proven by dropping @as from the declaration file for direction, and also double checking that assigning a random string to x on the second line still fails to type check.

The other issue behind that one is that by using #{}, you're claiming to LuaLS that all enum values are 0. Replace the second line in the above with x = 0 to see it. I think that worked before, but it seems that the goal is that it shouldn't. At the moment 0 is a valid value for any enum, even ones which are not integers (e.g. defines.events).

The event overloading also type checks wrong. I think it's related. Apparently @as is able to declare types, but their docs don't say it does. Also, removing:

---@overload fun(event:uint, handler:fun(event:EventData))

Fixes it locally, which might imply that at least part of LuaLS sees through the expressions and the overloads are effectively all 0 (which I would also expect to lead to a warning, so maybe not?) I haven't taken that further yet, but I will if you don't think it is likely to be related--you probably know LuaLS much better than us. That said it's worth noting that it does have to have some constant analysis or folding in it to work with unions of constants as a type so it does seem possible.

[justarandomgeek]

do

---@type defines.direction
local x = defines.direction.east
x = defines.direction.west

so that it doesn't deduce the narrower type based on the initialization.

the @as isn't actually defining the types, just masking away the dummy values to the enum member that it defines by name by existing as part of a table tagged with @enum, to make them opaque values. The #{} is actually treated as integer, not 0, and again, is simply to make them opaque values (as specified by factorio).

Having the enum members be separate individual types, and not just integer inside (iirc i even tried actually listing individual values, even though they're explicitly not supposed to be listed), was required for the overload resolution to work at all, but LuaLS is just still a bit twitchy about doing it correctly and sometimes snaps back to the params/returns tags instead, for reasons i failed to understand - but it seemed wrong to give up all the descriptions just for that. I was hoping for more upstream fixes to the overload resolution before trying to explore that further

[justarandomgeek]

and the uint overload for on_event is because taht's the type you get for custom events, but i had meant to ask that they just change that to use defines.events as the type for events everywhere (generate_event_name and such), even for unnamed members...

[ahicks92]

We could mark everything, but that's super annoying. Our mod has a lot of complexity, it's common for us to pick one of the directions as a default for example, then maybe flip it around. We're FactorioAccess, around 20kish lines and counting. If you specifically want to see the warnings we have you should be able to pull, git checkout next-update, and open vscode. Main is out of date, we don't work on it directly. Use main and it'll be horrifying because of legitimate problems.

Marking stuff once: sure. Doing it repeatedly for the indefinite future: that's annoying UX, especially for something that used to work (from the perspective of the IDE, anyway, even if typing as number maybe wasn't ideal).

I'd see the counterargument but overloads don't work on CustomInputEvent. Complains that player_index isn't present specifically. It might be a few other event types, I can audit if you want. We could mark those too, but...

From my perspective, it's not a trade-off because event overloads are flaky in at least one important case. I could see how most people would prefer to go that way if they worked 100%: I'd bet being as let us say algorithmic as we are is probably pretty rare.

At least one of the other devs downgraded to get away from it. I think the other may have as well. I haven't because my preference would be to fix it, whether that be here or on our side (also I became the volunteer for this issue, another reason to leave it). Sadly LuaLS docs leave much to be desired so other than explaining our problem I'm out of solutions to suggest.

If you are skeptical that our mod works, I'm blind and have won twice and will be streaming in the near future. I'm not trolling, I promise. If nothing else the code is there and as big as I say.

[EphDoering]

The LuaLS docs mention type narrowing issues in the overload annotation: https://luals.github.io/wiki/annotations/#overload which points to this issue: https://github.com/LuaLS/lua-language-server/issues/1456 So this does sound like an upstream issue, but I'm wondering how the plugin on_event module got around this.

[ahicks92]

@EphDoering I don't know that it did. In particular if LuaLS decides to typecheck as Any then it wouldn't complain and all we would observe is lack of autocomplete. Sometimes except also I've seen it be "smart" and offer things when things are typed as Any a few times, I could have sworn, so maybe that's not even enough. There's got to be a way to dump types; I haven't yet looked.

[justarandomgeek]

What does the Check Config command (from command palate) output?

[justarandomgeek]

The LuaLS docs mention type narrowing issues in the overload annotation: https://luals.github.io/wiki/annotations/#overload which points to this issue: LuaLS/lua-language-server#1456

This is a fairly old note from before the 3.9.* series that started actually implementing the overload resolution

... but I'm wondering how the plugin on_event module got around this.

the plugin does plain-text pattern matching and tag injection before LuaLS does the parse, but we're generally trying to keep those hacks to a minimum and retire them when possible since it's so much slower and more fragile than proper type-based solutions

[ahicks92]

I will get that for you later. Busy week. Doing a job hunt.

I don't believe there has been much movement on @overload. I did a serious dig through the internet via Google to try to find out wtf is going on before opening this and the best I could find was people complaining it's broken and the one open issue about @function.

I would expect LuaLS and any other capable type inference engine in a language with the concept to be able to unify these types by itself and recognize that it's the supertype (e.g. the "base class"). I would expect to at least see docs on overload resolution order or something. Apparently not? It's amazing how fragile it is around advanced concepts. But I guess that's OT.

I bet this also breaks the form on_event({ a, list, of, events }, handler...) but maybe that never worked.

Thanks for putting up with us. I know in some sense we're being picky.

[justarandomgeek]

https://marketplace.visualstudio.com/items/sumneko.lua/changelog see the initial entry for it under 3.9.0

[EphDoering]

sorry it took so long to get back to you. We have a launcher that uses Factorio's stdout that we're using as a layer between FMTK and Factorio and apparently I'm not getting the version check to get forwarded to FMTK's satisfaction. So while I figure out that issue here's the dump when I don't have our launcher in-between:

2024-06-29 18:20:17.496 [info] Check Config:
2024-06-29 18:20:17.506 [info] Active Factorio Version:
2024-06-29 18:20:17.564 [info] Binary: 1.1.107
2024-06-29 18:20:17.564 [info] Runtime JSON: 1.1.107
2024-06-29 18:20:17.596 [info] Library bundle OK in c:\Users\...\AppData\Roaming\Code\User\workspaceStorage\a0abab6ef892f43cd580f1486a98ca3d\justarandomgeek.factoriomod-debug, generated from Factorio 1.1.107 with FMTK 1.1.43
2024-06-29 18:20:17.596 [info] LuaLS 3.9.3 Activated
2024-06-29 18:20:17.596 [info] Lua.workspace.userThirdParty: workspace link OK (c:\Users\...\AppData\Roaming\Code\User\workspaceStorage\a0abab6ef892f43cd580f1486a98ca3d\justarandomgeek.factoriomod-debug\sumneko-3rd)
2024-06-29 18:20:17.596 [info] Lua.workspace.checkThirdParty = ApplyInMemory
2024-06-29 18:20:17.596 [info] Lua.workspace.library: /data link OK (c:\Program Files\Factorio\data)

[EphDoering]

Ok... so without the launcher in between now the type narrowing seems to be working....

Here's the dump when our launcher is in-between:

2024-06-29 18:39:53.530 [info] Active Factorio version: FA (1.1.107)
2024-06-29 18:41:25.943 [info] Check Config:
2024-06-29 18:41:26.091 [info] Active Factorio Version:
2024-06-29 18:41:29.254 [info] Binary: Error: Unable to read version
2024-06-29 18:41:29.255 [info] Runtime JSON: 1.1.107
2024-06-29 18:41:29.257 [info] Library bundle OK in c:\Users\...\AppData\Roaming\Code\User\workspaceStorage\a0abab6ef892f43cd580f1486a98ca3d\justarandomgeek.factoriomod-debug, generated from Factorio 1.1.107 with FMTK 1.1.43
2024-06-29 18:41:29.257 [info] LuaLS 3.9.3 Activated
2024-06-29 18:41:29.257 [info] Lua.workspace.userThirdParty: workspace link OK (c:\Users\...\AppData\Roaming\Code\User\workspaceStorage\a0abab6ef892f43cd580f1486a98ca3d\justarandomgeek.factoriomod-debug\sumneko-3rd)
2024-06-29 18:41:29.257 [info] Lua.workspace.checkThirdParty = ApplyInMemory
2024-06-29 18:41:29.257 [info] Lua.workspace.library: /data link OK (c:\Program Files\Factorio\data)

So looks like I need to figure out why my launcher isn't working with the version check.

[EphDoering]

Well, I got the version check to pass, and now the type narrowing seems to work about 30% of the time on my original Factorio Version configuration, but 100% of the time on the autodetected system version. The config check looks identical with either. After 3 hours of switching back and forth and getting inconsistent results I'm calling it a day.

[justarandomgeek]

well that is a fascinating (and baffling) result... 🤔

those config results look okay to me, they pretty much match what i've got here...

[ahicks92]

My config results are below, but @EphDoering all I have done is point fmtk at the folder, the launcher shouldn't get in the middle for me. Indeed later I actually ended up pointing it at a system install just to not have to deal with figuring all that out. Also I don't think fmtk launches it to get versions? Maybe it does.

Anyway:

2024-06-30 15:22:12.836 [info] FMTK 1.1.43
2024-06-30 15:22:12.836 [info] Registering Version Selector...
2024-06-30 15:22:12.836 [info] Registering Debug Provider...
2024-06-30 15:22:12.836 [info] Registering Language Client...
2024-06-30 15:22:12.836 [info] Registering Mod Package Provider...
2024-06-30 15:22:12.836 [info] Registering Profile Renderer...
2024-06-30 15:22:12.836 [info] Registering Custom Editors...
2024-06-30 15:22:12.836 [info] Activate Complete
2024-06-30 15:22:12.836 [info] Active Factorio version: System (1.1.107)
2024-06-30 15:22:44.319 [info] Check Config:
2024-06-30 15:22:44.387 [info] Active Factorio Version:
2024-06-30 15:22:44.422 [info] Binary: 1.1.107
2024-06-30 15:22:44.422 [info] Runtime JSON: 1.1.107
2024-06-30 15:22:44.457 [info] Library bundle OK in c:\Users\Austin\AppData\Roaming\Code\User\workspaceStorage\4d24b833eb0f247786d5164ff0fecd8b\justarandomgeek.factoriomod-debug, generated from Factorio 1.1.107 with FMTK 1.1.43
2024-06-30 15:22:44.457 [info] LuaLS 3.9.3 Activated
2024-06-30 15:22:44.458 [info] Lua.workspace.userThirdParty: workspace link OK (c:\Users\Austin\AppData\Roaming\Code\User\workspaceStorage\4d24b833eb0f247786d5164ff0fecd8b\justarandomgeek.factoriomod-debug\sumneko-3rd)
2024-06-30 15:22:44.458 [info] Lua.workspace.checkThirdParty = ApplyInMemory
2024-06-30 15:22:44.458 [info] Lua.workspace.library: /data link OK (c:\Program Files\Factorio\data)

sorry, got distracted and forgot I promised.

[justarandomgeek]

Also I don't think fmtk launches it to get versions?

i recently started launching with --version to check the binary version in Check Version and when starting a debug session, because a few people had messed up configs with a mismatched doc version when i started needing a reliable version number to control the unsafe debug switch

[ahicks92]

Hmm. Still don't think that should matter. Our launcher isn't named factorio.exe.

We do have a set of instructions for using our launcher to debug since it must be present in the chain for speech. I haven't set it up. Eph could be right being as they're the ones who designed how that works though, at least in their case.

[justarandomgeek]

can i see the workspace settings.json from one of you? chasing some issues with some other folks has revealed that Check Config isn't as thorough as it needs to be...

[EphDoering]

Here's mine:

{
    "folders": [
        {
            "path": "."
        },
        {
            "path": "../.."
        }
    ],
    "settings": {
        "Lua.workspace.library": [
            "c:\\Program Files\\Factorio\\data"
        ],
        "Lua.workspace.userThirdParty": [
            "c:\\Users\\...\\AppData\\Roaming\\Code\\User\\workspaceStorage\\a0abab6ef892f43cd580f1486a98ca3d\\justarandomgeek.factoriomod-debug\\sumneko-3rd"
        ],
        "Lua.workspace.checkThirdParty": "ApplyInMemory",
        "factorio.versions": [

            {
                "name": "FA",
                "configPath": "C:\\Users\\...\\Documents\\FA\\config\\config.ini",
                "factorioPath": "c:\\Program Files\\Factorio\\bin\\x64\\launcher_no_console.exe"
            },
            {
                "name": "System",
                "factorioPath": "C:\\Program Files\\Factorio\\bin\\x64\\factorio.exe",
                "active": true
            },
            {
                "name": "test",
                "configPath": "C:\\Users\\...\\Documents\\FA\\config\\config.ini",
                "factorioPath": "c:\\Program Files\\Factorio\\bin\\x64\\launcher_no_console.exe"
            }
        ]
    }
}

I was trying various combinations of changing the config file and the Factorio path, but I was getting inconsistent results and each time I switched something it took quite some time to analyze the whole workspace. The other thing was when I tried to make a minimal reproducible example in a new workspace with a test mod then it also seems to work every time.

My current hypothesis is that the LusLs has some timeout or race condition somewhere that's causing it to inconsistently narrow the function arguments.

[ahicks92]

I don't (yet) use workspace files. This is the relevant snippet of my user settings, something like 150 lines:

   "[lua]": {
      "editor.quickSuggestions": {
         "other": "off",
         "comments": "off",
         "strings": "off"
      },
      "editor.acceptSuggestionOnCommitCharacter": false,
      "editor.semanticHighlighting.enabled": true,
   },
   "factorio.versions": [
      {
         "name": "System",
         "factorioPath": "C:\\Program Files\\Factorio\\bin\\x64\\factorio.exe",
         "active": true
      }
   ],
   "Lua.workspace.userThirdParty": [
      "c:\\Users\\Austin\\AppData\\Roaming\\Code\\User\\workspaceStorage\\4d24b833eb0f247786d5164ff0fecd8b\\justarandomgeek.factoriomod-debug\\sumneko-3rd"
   ],

[justarandomgeek]

you must use workspace config, as the generated paths are workspace specific, and only the one for the current workspace gets updated.

alas, i was hoping to find that you simply had the duplicate userThirdParty entries i've seen a few others have cause similar trouble, but apparently not...

[justarandomgeek]

My current hypothesis is that the LusLs has some timeout or race condition somewhere that's causing it to inconsistently narrow the function arguments.

extremely plausible. it notably does lots of weird shit during its (multiple rounds of?) workspace-loading.

[ahicks92]

Ok, I thought you meant multi-folder workspace declaration files. That's my bad. I think this is what you want (.vscode/settings.json):

{
   "editor.insertSpaces": true,
   "editor.tabSize": 3,
   "Lua.diagnostics.globals": [
      "players",
      "printout",
      "get_selected_ent",
      "direction_lookup",
      "cursor_highlight",
      "sync_build_cursor_graphics",
      "get_direction_of_that_from_this",
      "clear_obstacles_in_rectangle",
      "ENT_TYPES_YOU_CAN_BUILD_OVER"
   ],
   "Lua.diagnostics.disable": [
      "lowercase-global",
      "deprecated",
      "need-check-nil"
   ],
   "Lua.workspace.library": [
      "c:\\Program Files\\Factorio\\data"
   ],
   "Lua.workspace.checkThirdParty": "ApplyInMemory",
   "Lua.runtime.version": "Lua 5.2",
   "[lua]": {
      "editor.defaultFormatter": "JohnnyMorganz.stylua"
   },
   "factorio.versions": [
      {
         "name": "System",
         "factorioPath": "C:\\Program Files\\Factorio\\bin\\x64\\factorio.exe",
         "active": true
      }
   ],
   "Lua.workspace.userThirdParty": [
      "c:\\Users\\Austin\\AppData\\Roaming\\Code\\User\\workspaceStorage\\4d24b833eb0f247786d5164ff0fecd8b\\justarandomgeek.factoriomod-debug\\sumneko-3rd"
   ]
}

That said unless you went out of your way to disable it, VSCode merges user settings should thus apply. I won't be coding for a few days but I can check that when I do. Docs say however that arrays replace and don't merge. Shouldn't be any trouble to comment out and check though.

[justarandomgeek]

hrm. that looks like it should work...🤔