luttje / glua-api-snippets

Scrapes the Garry's Mod Wiki in order to build Lua Language Server comments that will provide IDE suggestions and autocompletion.
MIT License
15 stars 5 forks source link

Global tables (like DVScrollBar, GM and BaseClass) aren't exposed globally #50

Open TankNut opened 3 months ago

TankNut commented 3 months ago

I'm not sure what exactly is going on here but the glua definitions for panels are doing some weird things resulting in functions going missing.

Snippet from sumneko.lua\addonManager\addons\garrysmod\module\library\dvscrollbar.lua updated fresh from the addon manager:

---@meta

---[CLIENT AND MENU] Sets the scroll level in pixels.
---
---[(View on wiki)](https://wiki.facepunch.com/gmod/DVScrollBar:SetScroll)
---@param scroll number The new scroll value.
function DVScrollBar:SetScroll(scroll) end

---[CLIENT AND MENU] Returns the amount of scroll level from the top in pixels
---
---[(View on wiki)](https://wiki.facepunch.com/gmod/DVScrollBar:GetScroll)
---@return number # The amount of scroll level from the top
function DVScrollBar:GetScroll() end

---@class DVScrollBar : Panel
local DVScrollBar = {}

Those two functions are placed above the class annotation and are throwing undefined-field diagnostic errors as a result.

As a separate but related issue, global tables like DVScrollBar, GM and BaseClass are defined as locals in the snippet files, requiring people to manually add them to diagnostics.globals which seems unnecessary.

TankNut commented 3 months ago

After some more digging it seems that the Lua Addon Manager version is behind on releases and most (excluding the panel globals) of the issues here seem to be resolved. Sorry about that!

luttje commented 3 months ago

Ouch, thanks for reporting! I should update the Lua Addon Manager version. I'll get on it

luttje commented 3 months ago

The LLS-addon should be updated soon, I made a PR at their repo.

As for your second point:

As a separate but related issue, global tables like DVScrollBar, GM and BaseClass are defined as locals in the snippet files, requiring people to manually add them to diagnostics.globals which seems unnecessary.

This is a good point, I never noticed this before. I'll leave this issue open and rename it

Actually wait, before I continue that thought further:

Am I misunderstanding what you're saying and are you sure there is a problem?

Thanks for clarifying 👍

TankNut commented 3 months ago

From what I remember in Gmod, GM is only available for a short while during initialization, after which GAMEMODE or gmod.GetGamemode give you the gamemode reliable at runtime (I could be wrong)

While GM isn't a conventional global it is always going to be available during file load and if you do anything related to gamemode development you will be using it similarly to how SWEP, ENT, and EFFECT work. Though more on those three in a moment.

And baseclass is exposed too (BaseClass is usually specified as a local, as per the wiki example)

I guess this is less of a problem of BaseClass not being a global and more the snippets not supporting the common use case of DEFINE_BASECLASS("weapon_hands") which is internally translated (as per the wiki example) to local BaseClass = baseclass.Get("weapon_hands")

It's not a big loss but it does require people to switch their code over to manually doing the translations ahead of time.

[...] similar to how SWEP, ENT, and EFFECT work.

Unfortunately this ended up being the dealbreaker for me and is why I've stopped using the language server for the time being. The issue stems from all three of those tables being defined and undefined at different points during the loading process in a way that doesn't seem to be compatible with how they are currently defined as globals. To give an example:

-- <file 1>
SWEP.PrintName = "Gun"

function SWEP:CustomFunction()
end

-- <file 2>
SWEP.PrintName = "More Gun"

function SWEP: -- Will suggest CustomFunction, despite not being a function defined on this SWEP

This combination will also result in hovering over SWEP.PrintName listing out

(global) SWEP.PrintName: string = "Gun"|"More Gun"

which becomes nearly impossible to manage at any sort of scale that would warrant the language server in the first place as you're likely to have dozens of weapons and entities that all end up polluting the same global table.

I'm not sure what the solution to this could be and whether a solution is possible at all.

luttje commented 3 months ago

Thanks for clarifying, that helps a lot! And I can't believe I pointed to a wiki example and then didn't read it 😅

Perhaps the easiest one to fix on the short term is GM? Since that can just be global, without causing conflicting code suggestions.

I'll have a think about this, since exposing all of them globally causes the problem you described.

TankNut commented 3 months ago

GM along with the derma ones (some clarification on that, derma.DefineControl creates a global pointing at the PANEL table and is used for all of the built-in panels) should be fairly straightforward fixes yeah.