Skip to content

Add missing names to lua return values. Additional return description and overload cleanup. - #3170

Open
sparr wants to merge 1 commit into
beyond-all-reason:masterfrom
sparr:lua_comment_fixes
Open

Add missing names to lua return values. Additional return description and overload cleanup.#3170
sparr wants to merge 1 commit into
beyond-all-reason:masterfrom
sparr:lua_comment_fixes

Conversation

@sparr

@sparr sparr commented Aug 3, 2026

Copy link
Copy Markdown

The generated Lua types have a lot of @return comments with descriptions but no names. LuaLS/LuaCATS requires either a name or a # before the description. I opted to add names and attempted to follow the existing naming conventions. While I was there (read: looking at search results for @return in Lua*.cpp) I also made what I think are a few other improvements to typings and overloads.

https://luals.github.io/wiki/annotations/#return is the relevant documentation for the type annotations.

I discussed this a little on the Beyond All Reason Discord, starting around https://discord.com/channels/549281623154229250/549282166543089674/1533836286358458428. I tried to find the right channel on the Recoil Engine Discord but haven't had much success yet.

Screenshot of VSCode Lua extension (which uses LuaLS) interpreting the first word of the description as the name of the return value: image
After:image

Comment thread rts/Lua/LuaOpenGL.cpp
* @function gl.GetMatrixData
* @param type GL Matrix type (`GL.PROJECTION`, `GL.MODELVIEW`, `GL.TEXTURE`).
* @return Matrix4x4 The matrix.
* @return Matrix4x4 matrix The matrix.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These got a bit too redundant

Comment thread rts/Lua/LuaOpenGL.cpp
* @function gl.GetMatrixData
* @param name MatrixName The matrix name.
* @return Matrix4x4 The matrix.
* @return Matrix4x4 matrix The matrix.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These got a bit too redundant

*
* @return boolean allowed
* @return number the new priority for this target (if you don't want to change it, return defPriority). Lower priority targets are targeted first.
* @return number newPriority the new priority for this target (if you don't want to change it, return defPriority). Lower priority targets are targeted first.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably:

Suggested change
* @return number newPriority the new priority for this target (if you don't want to change it, return defPriority). Lower priority targets are targeted first.
* @return number newPriority The new priority for this target (if you don't want to change it, return defPriority). Lower priority targets are targeted first.

Comment thread rts/Lua/LuaSyncedRead.cpp
* @param unitID integer
*
* @return table<string,number> where keys are piece names and values are piece indices
* @return table<string,number> pieceInfos where keys are piece names and values are piece indices

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a pieceMap?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this name is used multiple other places

* @function Spring.GetSelectedUnitsSorted
* @return table<number,number[]> where keys are unitDefIDs and values are unitIDs
* @return integer the number of unitDefIDs
* @return table<number,number[]> unitsIDs where keys are unitDefIDs and values are unitIDs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe unitDefMap, unitsByDefID?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to follow the convention from unitsCounts.

* @return table<number,number[]> where keys are unitDefIDs and values are unitIDs
* @return integer the number of unitDefIDs
* @return table<number,number[]> unitsIDs where keys are unitDefIDs and values are unitIDs
* @return integer count the number of unitDefIDs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

countDefs?

*
* @return table<number,number> unitsCounts where keys are unitDefIDs and values are counts
* @return integer the number of unitDefIDs
* @return integer count the number of unitDefIDs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

countDefs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants