Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add new functions: pathListDir, pathIsFile, pathIsDirectory #3189

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

Merged
Dutchman101 merged 31 commits into multitheftauto:master from TracerDS:180923_Add-fileListDir
May 25, 2024
Merged

Add new functions: pathListDir, pathIsFile, pathIsDirectory #3189

Dutchman101 merged 31 commits into multitheftauto:master from TracerDS:180923_Add-fileListDir
May 25, 2024

Conversation

@TracerDS
Copy link
Contributor

@TracerDS TracerDS commented Sep 18, 2023
edited
Loading

Also refactored SharedUtil a little (WIN32 might not be defined. _WIN32 will always be defined)

Syntax: (削除) table fileListDir ( string path ) (削除ここまで) table pathListDir ( string path )
returns table of entries on success, nil on failure

Additional functions

(削除) bool isFile ( string path ) (削除ここまで) bool pathIsFile ( string path )
Returns true if path is a file, false if a directory or path doesn't exist

(削除) bool isDirectory ( string path ) (削除ここまで) bool pathIsDirectory ( string path )
Returns true if path is a directory, false if a file or path doesn't exist

OOP functions

path.listDir => pathListDir
path.isFile => pathIsFile
path.isDirectory => pathIsDirectory


Resolves: #346

Xenius97, Fernando-A-Rocha, onikyannn, curukcesetgurmesi, Bence58, sacr1ficez, Shuubaru, Nico8340, Wannacry-ops, Loki214, and CrosRoad95 reacted with heart emoji Nico8340 and Fernando-A-Rocha reacted with rocket emoji
@lopezloo lopezloo added the enhancement New feature or request label Sep 19, 2023
Copy link
Contributor

Pirulax commented Sep 20, 2023

Is this supposed to return a list of files in a directory?

Copy link
Contributor Author

Is this supposed to return a list of files in a directory?

Relative to a current resource, yes

Copy link
Member

botder commented Oct 2, 2023

Is the Unix code block functional with if (!(dir = opendir("c:\\src\\")))?
Also, don't you exclude the directory items in the < C++17 block?
And last, how can the scripter differentiate between file and directory?

Copy link
Contributor Author

TracerDS commented Oct 5, 2023

Is the Unix code block functional with if (!(dir = opendir("c:\\src\\")))? Also, don't you exclude the directory items in the < C++17 block? And last, how can the scripter differentiate between file and directory?

  1. Changed path to the actual path. Should be functional now
  2. Now including all files in a directory without adding the directory name before it too
  3. via isFile and isDirectory functions (comment edited)

Copy link
Member

botder commented Oct 8, 2023

Wouldn't it make more sense to append a directory separator for directory items?
Checking each item whether it's a directory or file seems expensive?

Copy link
Contributor Author

TracerDS commented Oct 8, 2023
edited
Loading

Wouldn't it make more sense to append a directory separator for directory items?

It might cause some issues when messing with directories directly via that function (unless mta takes care of that already)

Checking each item whether it's a directory or file seems expensive?

No, it's not really expensive.

If MTA doesn't have problems with directories ending with \ | / then sure, I can add a separator for directory entries

Copy link
Contributor

Pirulax commented Oct 14, 2023

I don't think fileListDir makes a lot of sense as a name.
Perhaps something like getDirectoryEntries or (or, I'd prefer to separate them, getDirectoryFiles and getDirectoryFolders, both with a bool recursive parameter as well.
Or, could even go with pathGetFolders, pathGetFiles [if we want to follow the file prefix convention thingy]

xAzke reacted with thumbs up emoji

Copy link
Contributor Author

I don't think fileListDir makes a lot of sense as a name. Perhaps something like getDirectoryEntries or (or, I'd prefer to separate them, getDirectoryFiles and getDirectoryFolders, both with a bool recursive parameter as well. Or, could even go with pathGetFolders, pathGetFiles [if we want to follow the file prefix convention thingy]

its better to have one function that gets all entries that you can sort by using isFile/isDirectory functions.

Copy link
Member

DNL291 commented Oct 28, 2023

There's the module FileSystem which already does the same and has some more features/functions btw.
Although could be added nativelly yes if the only purpose is to get files from a directory instead of using the module just for that.

Copy link
Contributor Author

There's the module FileSystem which already does the same and has some more features/functions btw.
Although could be added nativelly yes if the only purpose is to get files from a directory instead of using the module just for that.

Using module for that will decrease performance (since you have to load the module, load its functions and events not to mention modules are very limited and you cant use almost any MTA functions there at all without refactoring the whole mta server).
Also that would only make the command work in server scripts only.

Implementing it directly in MTA solves all these issues

Copy link
Member

DNL291 commented Oct 28, 2023

There's the module FileSystem which already does the same and has some more features/functions btw.
Although could be added nativelly yes if the only purpose is to get files from a directory instead of using the module just for that.

Using module for that will decrease performance (since you have to load the module, load its functions and events not to mention modules are very limited and you cant use almost any MTA functions there at all without refactoring the whole mta server). Also that would only make the command work in server scripts only.

Implementing it directly in MTA solves all these issues

Uhm I'm not sure if we really have these issues in the (FileSystem) module. But I do agree that having the functions client-side and being directly implemented into MTA is better.

Fernando-A-Rocha reacted with thumbs up emoji

Copy link
Member

tederis commented Oct 29, 2023
edited
Loading

There's the module FileSystem which already does the same and has some more features/functions btw.
Although could be added nativelly yes if the only purpose is to get files from a directory instead of using the module just for that.

Using module for that will decrease performance (since you have to load the module, load its functions and events not to mention modules are very limited and you cant use almost any MTA functions there at all without refactoring the whole mta server). Also that would only make the command work in server scripts only.

Implementing it directly in MTA solves all these issues

I doubt that modules can affect performance in a context of filesystem functions. The main disadvantage that modules have - they don't exist on a client side. So the only way to add proposed in this PR functions is to make them a part of MTA.

But I personally would not say that these functions are really needed on a client side. Typically, server(and therefore client scripts) know which files exist on a client and how to reach them, which makes sense from the security perspective.

DNL291 and salwador reacted with thumbs up emoji

@TracerDS TracerDS changed the title (削除) Add new function: fileListDir (削除ここまで) (追記) Add new function: pathListDir (追記ここまで) Feb 2, 2024
@TracerDS TracerDS changed the title (削除) Add new function: pathListDir (削除ここまで) (追記) Add new functions: pathListDir, pathIsFile, pathIsDirectory (追記ここまで) Feb 2, 2024
Lpsd and others added 13 commits February 5, 2024 16:41
VS2022: 17.6.33815.320 => 17.6.33829.357
[skip ci]
This is an automated commit to keep track of toolchain changes on the build server.
It applies to every MTA build after this commit until further notice.
VS2019: 16.11.33801.447 => 16.11.33920.266
[skip ci]
This is an automated commit to keep track of toolchain changes on the build server.
It applies to every MTA build after this commit until further notice.
VS2019: 16.11.33927.289 => 16.11.34031.81
[skip ci]
This is an automated commit to keep track of toolchain changes on the build server.
It applies to every MTA build after this commit until further notice.
Build Tools 2022 (17.7.34031.279)
This is an automated commit to keep track of toolchain changes on the build server.
It applies to every MTA build after this commit until further notice. [skip ci]
Also refactored SharedUtil a little (`WIN32` might not be defined. `_WIN32` will always be defined)
Now `SharedUtil::ListDir` is crossplatform/crossservice
TracerDS added 3 commits March 25, 2024 19:02
Cannot remove `#if __cplusplus >= 201703L` because client side code is using these functions (multiplayer_sa is c++14)
`WIN32_TESTING` was changed to `_WIN32_TESTING`. Changed back to the original definition
Copy link
Member

@Nico8340 Nico8340 left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

Copy link
Member

There's been many code reviews, so merging.
@TracerDS thanks, please add it to wiki though

TracerDS reacted with heart emoji

@Dutchman101 Dutchman101 merged commit 74781c6 into multitheftauto:master May 25, 2024
MTABot pushed a commit that referenced this pull request May 25, 2024
74781c6 Add new functions: `pathListDir`, `pathIsFile`, `pathIsDirectory` (#3189)
@TracerDS TracerDS deleted the 180923_Add-fileListDir branch May 26, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheNormalnij TheNormalnij Awaiting requested review from TheNormalnij

@tederis tederis Awaiting requested review from tederis

@botder botder Awaiting requested review from botder

1 more reviewer

@Nico8340 Nico8340 Nico8340 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Getting files and directories in specified path

AltStyle によって変換されたページ (->オリジナル) /