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

Directory: Alternative to list_dir_begin: get_files #1225

Closed
nathanfranke opened this issue Jul 20, 2020 · 5 comments · Fixed by godotengine/godot#40547
Closed

Directory: Alternative to list_dir_begin: get_files #1225

nathanfranke opened this issue Jul 20, 2020 · 5 comments · Fixed by godotengine/godot#40547
Milestone

Comments

@nathanfranke
Copy link

Describe the project you are working on:
Working on some tests following my recent pull requests on DirAccess bug fixes.

Describe the problem or limitation you are having in your project:
Listing files in a directory requires a lot of boilerplate code.

Example A:

dir.list_dir_begin()

var file = dir.get_next()
while not file.empty():
	print(file)
	file = dir.get_next()

dir.list_dir_end()

Example B:

dir.list_dir_begin()

var file
while true:
	file = dir.get_next()
	if not file:
		break
	print(file)

dir.list_dir_end()

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
This proposal will allow an alternative option that is a lot simpler to write.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

for file in dir.list_files():
	print(file)

Note 1: If buffering all the files in a list is an issue, we should probably change file_access_pack
image

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Technically yes, this can be worked around by writing one of the top examples. However, this is used quite often and will not break compatibility for 3.2.

Note 2: Depending on how people think of this, list_dir_begin, get_next, and list_dir_end could be deprecated or removed in 4.0.

Is there a reason why this should be core and not an add-on in the asset library?:
This is not really relevant since Directory is already part of core.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 20, 2020

Yeah, I have some code written to work with EditorFileSystemDirectory for editor plugins, including the mentioned get_files method. Similarly you could write get_files_recursive() just like there's make_dir_recursive() and whatnot. But the core functionality will remain simplistic anyways whatever we propose, so I'm for improving the process of traversing the files within a directory using "for loop" at least.

As you can see there, EditorFileSystemDirectory has the methods like get_file_count() and get_file(). At the very least, the Directory class API could be made consistent with EditorFileSystemDirectory on some level.

@nathanfranke
Copy link
Author

We could also implement an iterator class for GDScript so there is no buffering issue

@Xrayez
Copy link
Contributor

Xrayez commented Jul 20, 2020

@nathanfranke
Copy link
Author

nathanfranke commented Jul 20, 2020

"git gut" :P

@KoBeWi
Copy link
Member

KoBeWi commented Jul 20, 2020

I looked at DirAccess code and the funny thing is that it already stores internally the list of files/directories and get_next() just pops elements from this list xd

EDIT:
Ok, nevermind. I actually looked at one of the implementations. The method is abstract.

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

Successfully merging a pull request may close this issue.

5 participants