Joe91 / fun-bots

A highly customizable and configurable bot mod for your Venice Unleashed Battlefield 3 server.
https://discord.gg/zNk3qCzk6x
237 stars 45 forks source link

feat: `scan_for_invalid_comments` implementation #253

Closed joao-vitor-souza closed 1 year ago

joao-vitor-souza commented 1 year ago

Guys, I built a comment "linter" for Lua files. You need to run fun-bots-helper.py to acess it, it's the Fix Grammar button:

image

Pressing this button, all¹ Lua files will be scanned and their comments (everything after --²) will be formatted and grammarly checked. For Instance, if there is this comment:

--this is a comment
-- wait, what is these?
--resorce isnt neededd
-- A coomment with a camelCase variable wont check the variable: self.PlayerCount

The linter post request an API and give this result:

-- This is a comment. 
-- Wait, what are these? 
-- Resorce isn't needed. 
-- A comment with a camelCase variable won't check the variable: self.PlayerCount.

It still has some flaws, but this is the first version. Certainly it will help to standardize future code documentations.

I already scanned and reviewed all files, the results were commited here.

Also, I found some comments saying that something is unused or should be removed, maybe it's worth taking a look at them:

NodeCollection.lua -> Line 1447
NodeEditor.lua -> Lines 604 and 963
UIServer.lua -> Line 26
Vehicles.lua -> Line 191
Bot.lua -> Line 226
Database.lua -> Lines 27, 96 and 139
SettingsManager.lua -> Line 154

¹ SettingsDefinition.lua, Config.lua and ext/Shared/Languages gave weird results, so I checked them manually. Next scans won't search for them. ² Multiple line comments (--[[]]) are not working yet.

joao-vitor-souza commented 1 year ago

The linter ends all comments with a space `, this is a flag indicating that the comment were already checked. If you don't want the linter to check a comment, just add ` at the end of it.

Joe91 commented 1 year ago

Wow! Great stuff! Looks really good! Only thing I'v noticed: the auto-generated files like the Config.lua should not be checked or we should create the file with the spaces in it. What would you suggest? Everything else looks fine! I think really only the Config.lua is missing. So lets just add the spaces in the generation of it?

joao-vitor-souza commented 1 year ago

Wow! Great stuff! Looks really good! Only thing I'v noticed: the auto-generated files like the Config.lua should not be checked or we should create the file with the spaces in it. What would you suggest? Everything else looks fine! I think really only the Config.lua is missing. So lets just add the spaces in the generation of it?

Yep, I noticed that, I'll fix the generation of it.

Joe91 commented 1 year ago

Seems like some coments just get deleted when I run the script on windows. Could be related to the encoding? In the latest dev-commit i lose some lines in pathSwithcer.lua for example: grafik

joao-vitor-souza commented 1 year ago

Seems like some coments just get deleted when I run the script on windows. Could be related to the encoding? In the latest dev-commit i lose some lines in pathSwithcer.lua for example: grafik

Weird, will find out and commit the changes.

Joe91 commented 1 year ago

No hurry at all. It seems to only happen on windows or on my system. Can investigate this also myself a little more tomorrow evening. Just noticed it after I run the script again on my merged files...

joao-vitor-souza commented 1 year ago

No hurry at all. It seems to only happen on windows or on my system. Can investigate this also myself a little more tomorrow evening. Just noticed it after I run the script again on my merged files...

Found the error, I forgot to append a line when no correction was returned from the API, I continue to the next line instead appending it the way it is first. Now it's fixed!

except IndexError:
    out_file_lines.append(line)
    continue
Joe91 commented 1 year ago

Ah makes sense. So did we lose some comments then with the first merge? Will go through them this evening. Thanks for fixing it!

joao-vitor-souza commented 1 year ago

Ah makes sense. So did we lose some comments then with the first merge? Will go through them this evening. Thanks for fixing it!

Nope, I manually checked every single line before the first merge and restore those that were deleted (a few of them). If you'd run it before, it'd delete them again (as you did). Not any more.

Joe91 commented 1 year ago

Hey, are you online on discord? The change with the settings-definition is not that good in my opinion. Let us discuss that there?