Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RDY] Luacheck inlining #2152

Merged
merged 10 commits into from Jul 3, 2022
Merged

[RDY] Luacheck inlining #2152

merged 10 commits into from Jul 3, 2022

Conversation

tobylane
Copy link
Member

@tobylane tobylane commented May 4, 2022

Describe what the proposed change does

  • Rewrite a few ifs to avoid empty branches
  • Add inline ignores for some types of warnings

These commits can be squashed, they're only this way for some clarity.

Copy link
Member

@lewri lewri left a comment

Choose a reason for hiding this comment

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

Nice cleaning

-- W212: unused argument XYZ
-- W231: variable XYZ is never accessed
-- W542: empty if branch
Copy link
Member

Choose a reason for hiding this comment

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

I'd keep these comment lines incase they end up being needed in the future

Copy link
Member

@TheCycoONE TheCycoONE May 6, 2022

Choose a reason for hiding this comment

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

That's what luacheck's documentation is for - https://luacheck.readthedocs.io/en/stable/warnings.html

I agree with removal.

CorsixTH/Lua/graphics.lua Show resolved Hide resolved
CorsixTH/Lua/world.lua Outdated Show resolved Hide resolved
CorsixTH/Lua/window.lua Show resolved Hide resolved
@@ -46,7 +46,7 @@ local function action_seek_reception_start(action, humanoid)

-- Go through all receptions desks.
for _, desk in ipairs(humanoid.hospital:findReceptionDesks()) do
if (not desk.receptionist and not desk.reserved_for) then
Copy link
Member

@TheCycoONE TheCycoONE May 8, 2022

Choose a reason for hiding this comment

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

This is a good candidate to rewrite. if desk.receptionist or desk.reserved_for then

@lewri
Copy link
Member

lewri commented May 8, 2022

Just a thought, but if you're leaning towards luacheck inlining (actually gives the ignore context imo) maybe it would be noted in the .luacheck file that this method should be preferred

@tobylane
Copy link
Member Author

I rewrote some of the simpler empty ifs, the others will come later.

There's now a note on the wiki page for Coding Conventions, and I'll add a line to luacheckrc when that bottom section is cleared.

@@ -811,7 +811,7 @@ end
--!return (array of {xpos, ypos} tables) Coordinates to draw the selected sprite.
function UIMapEditor:getDrawPoints()
if self.cursor.state == "disabled" then
-- Nothing to compute, drop to bottom {} return.
return {} -- Nothing to compute, return early
Copy link
Contributor

Choose a reason for hiding this comment

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

Use 2 space indenting.

@tobylane tobylane changed the title Luacheck inlining [RDY] Luacheck inlining May 19, 2022
@tobylane
Copy link
Member Author

Ready.

@TheCycoONE
Copy link
Member

Holding due to release

@TheCycoONE TheCycoONE merged commit 7a8ca36 into CorsixTH:master Jul 3, 2022
@tobylane tobylane deleted the luacheckinline branch October 24, 2023 17:21
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.

None yet

4 participants