Convert LDoc to lua-language-server

[rhys-vdw]

Goal

Completely convert over to lua-language-server compatible Lua type annotations that can be imported into BAR and other projects that use Recoil.

Plan has been discussed with @badosu. It's going to take a little while.

Steps

• Use sed to bulk convert annotations script here
• Complete the remainder manually.
• Create doc extractor tool lua-doc-extractor
• Support for enum generation rhys-vdw/lua-doc-extractor#1
  • LuaConstCMD.cpp CMD enum
  • LuaConstCMDTYPE.cpp CMDTYPE enum
• Complete various other doc extractor issues
• Add luarc.json to spring
• Update online documents to render JSON from LLS document export.
• GitHub action automation (when any PR touches rts/Lua/*.cpp).
  • Regenerate Lua metafiles.
  • Regenerate online docs.
• Create an LLS library that can be depended on by BAR
  • Create submodule Here: https://github.com/rhys-vdw/recoil-lua-library/
  • Keep generated docs in submodule and update it with workflow
• Create a style and usage guide somewhere.
• Add workaround for LuaLS autodoc is including global table in output md/json files LuaLS/lua-language-server#2977

Submodule approach:

• Restore submodule support
• Remove generated library files from this repository
• Support hand-written files (keep in this repo, but duplicate into submodule)

Follow up:

• Move repo lua-doc-extractor into BAR org
• Get admin privileges (since it's all my code)
• Move library repo, and update submodule links.
• Update all workflows and readme links to new URL

Post-merge steps:

• Update Add generated engine docs and types for Lua Language Server Beyond-All-Reason#3949 so to use submodule.
• Consider adding library module to https://github.com/LuaLS/LLS-Addons/

Stuff to follow up on

• What types are these
  

  spring/rts/Lua/LuaFBOs.cpp

  Lines 412 to 415 in 38598d1

  	/***
  	* @table attachment
  	* attachment ::= luaTex or `RBO.rbo` or nil or { luaTex [, num target [, num level ] ] }
  	*/

• What table are these LuaHandle functions on?
  

  spring/rts/Lua/LuaHandle.cpp

  Lines 490 to 493 in 38598d1

  	/*** Called when the game is (re)loaded.
  	*
  	* @function LoadCode
  	*/

• Same as LuaMenu ☝️
• GetSolidObjectPieceInfoHelper returns [x,y,z] and other things return { x:, y:, z: }. Come up with good names for them (currently latter is float3).
• PieceInfo class to use float3 type
• Same with rgb in LuaUnsyncedCtrl::SetAtmosphere, this is an array, but would it also support a table?
• Centralize shared types into their own file.
• Two different and conflicting definitions of cmdOpts — one claims that it can be provided as an array of values, but has different fields — confirm which is true and whether array is supported (and which params are supported in this array)
  • LuaUnsyncedCtrl.cpp
  • LuaHandle.cpp
• Same as cmdSpec ☝️ (this is the same, but has the nested cmdOpts so is it the same?
• Possible error in LuaUnsyncedCtrl::GiveOrder — returns 1 without pushing return value?
• Unify GLenum type name and table name (GL). Bit confusing as is!
• Do a final search for @func x and replace with @param x function
• Consider defining a float type?
• LuaZip.cpp seems to be completely unused, it has some methods on Spring and VFS and others on a table called ZipFileWriter but they're not referenced anywhere in Recoil in BAR.
• Lots of duplication in LuaConstGL.cpp
• Docs look completely wrong for:
  • CLuaHandle::UnitCommand
  • CLuaHandle::UnitCmdDone
• LuaOpenGL.cpp is mostly undocumented.

Manual conversion checklist

• LuaBitOps.cpp
• LuaConstCMD.cpp (blocked by enums)
• LuaConstCMDTYPE.cpp (blocked by enums)
• LuaConstCOB.cpp
• LuaConstEngine.cpp
• LuaConstGL.cpp
• LuaConstGame.cpp
• LuaConstPlatform.cpp
• LuaFBOs.cpp
• LuaHandle.cpp
• LuaHandleSynced.cpp
• LuaMathExtra.cpp
• LuaMenu.cpp
• LuaMetalMap.cpp
• LuaOpenGL.cpp
• LuaRBOs.cpp
• LuaRules.cpp
• LuaShaders.cpp
• LuaSyncedCtrl.cpp
• LuaSyncedMoveCtrl.cpp
• LuaSyncedRead.cpp
• LuaUnsyncedCtrl.cpp
• LuaUnsyncedRead.cpp
• LuaVAO.cpp
• LuaVAOImpl.cpp
• LuaVBO.cpp
• LuaVBOImpl.cpp
• LuaVFS.cpp
• LuaZip.cpp

[rhys-vdw]

Is there a way to stop CI from running while the PR is in draft? I didn't realize it was going to keep spinning up CI every time I pushed.

[sprunk]

In that case check the other files because sometimes you do @param type name and sometimes @param name type.

spring/rts/Lua/LuaBitOps.cpp

Lines 61 to 62 in f9d2bc7

	* @param a1 integer
	* @param a2 integer

spring/rts/Lua/LuaSyncedRead.cpp

Lines 1346 to 1347 in f9d2bc7

	* @param number x
	* @param number z

[rhys-vdw]

In that case check the other files because sometimes you do @param type name and sometimes @param name type.

Sorry it's really going to take a while, I have a full time job. I spent the entire weekend writing the code extractor, and then did a first pass with sed. I'm quite motivated to get it all done, but you'll need to wait a little bit. There's only so much that can be achieved with regex replace!

Happy to receive feedback, but pls focus on the files I've checked off in the folded up "manual conversion checklist" in the PR description.

[rhys-vdw]

Hm, actually you're right. The initial sed pass did output the @param args in the wrong order... Hm! I might fix it and rebase the changes in. Probably will be a net time saving.

[rhys-vdw]

In that case check the other files because sometimes you do @param type name and sometimes @param name type.

Good catch @sprunk. I've updated the regex and rebase the other commits on top. 9696f51

[rhys-vdw]

I've opened a draft PR on BAR repo with my latest generated docs: beyond-all-reason/Beyond-All-Reason#3949

[badosu]

I wonder if we can convince lua-ls to actually define the constant with the number we already know

[rhys-vdw]

This was generated by my lua-doc-extractor, so we do have full control over how it is generated.

It is actually defined as that constant. Constants are types in LLS, so @type -1 is correctly defining the value, LuaLS then disregards the nil. I can't remember exactly why I did it this way, but I think it was just easier to generate and seems to work fine.

However, I noticed that this style of enum declaration (marking a table as an enum) does not export to docs properly (it ends up just knowing there is an enum called CMD, but is not aware of its members). I believe it is "correct" in terms of the Lua interface, given that there is indeed a table called CMD defined in the global table. So I'd probably prefer to take this issue up with LuaLS maintainers and have it fixed upstream rather than screw around with the generation to make it conform.

[rhys-vdw]

rhys-vdw/lua-doc-extractor#9

[rhys-vdw]

@badosu I removed lua-ldoc and lua-markdown, but maybe none of these are needed anymore?

[badosu]

Should not be

[rhys-vdw]

@badosu pls see changes made here. github.ref_name should be the branch that triggered the merge. This allows testing of this workflow from an alternative branch during development.

The removal of ref: from actions/checkout also will default to the action branch.

In general I expect this will still run on master when executed by cron.

[rhys-vdw]

That said, it might make sense to run this job when master changes rather than on a timer? I also noticed that it's running on my fork (and presumably every other fork), and just failing every time.

[badosu]

I'd rather have it on a timer simply due to the fact it's not critical docs are updated more frequently than daily, but we have more updates than once a day. But this is my opinion.

[saurtron]

here are some ldoc strings

[saurtron]

I think it'd be better to not have autogenerated stuff in the same repo, maybe a different repo linked through submodule could have a similar effect.

I think it would pollute the git history here, make the repo larger, complicate checking diff between commits a bit, and eventually may cause other problems.

Also for linking into other repos, like game repos, I think submodule approach will make it easier for them too, otherwise even if they can restrict to having just a subfolder in the working tree they probably will need to deal with the whole recoil repo and history and keep it around to be able to update.

Not saying this is a blocker, just a concern.

[saurtron]

I think it'd be better to not have autogenerated stuff in the same repo

Otherwise at least maybe don't put it under rts/ (or cont/) since that's where cpp/lua code lives and it will complicate text searching there if we have autogenerated stuff.

[badosu]

Otherwise at least maybe don't put it under rts/ (or cont/) since that's where cpp/lua code lives and it will complicate text searching there if we have autogenerated stuff.

Sounds good, maybe inside doc?

[rhys-vdw]

I think it'd be better to not have autogenerated stuff in the same repo, maybe a different repo linked through submodule could have a similar effect.

I think it would pollute the git history here, make the repo larger, complicate checking diff between commits a bit, and eventually may cause other problems.

Also for linking into other repos, like game repos, I think submodule approach will make it easier for them too, otherwise even if they can restrict to having just a subfolder in the working tree they probably will need to deal with the whole recoil repo and history and keep it around to be able to update.

Not saying this is a blocker, just a concern.

I agree with @saurtron here, but my primary concern is with BAR requiring the entire history of this repo just for a few files. My original approach was to keep the lua files here (generated by the workflow) but then have them copied into an additional repo that can be used as a submodule by BAR (and any other engine consumer).

This is primarily because there is support hand-authored lua library files (currently just types.lua), which really ought to be committed with any changes to the CPP docs into this repo (but definitely belong in the submodule). It is not derived from other CPP files.

I think maybe these hand-authored library files can live in rts/Lua/library, ignored from the lua config file, and copied over into the submodule when the rest is generated.