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

Make some methods in File and Directory class static #1101

Closed
Tracked by #15
KoBeWi opened this issue Jun 21, 2020 · 6 comments
Closed
Tracked by #15

Make some methods in File and Directory class static #1101

KoBeWi opened this issue Jun 21, 2020 · 6 comments
Milestone

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Jun 21, 2020

Describe the project you are working on:
Multiple projects using files API for file operations.

Describe the problem or limitation you are having in your project:
I do lots of file operations. While opening files and iterating directories is ok, there's one thing that always makes me cringe: checking if file exists. I can't do File.file_exists(path). I have to

var file_checker = File.new()
if file_checker.file_exists(path)

or more compact version (when I don't need to use it multiple times) File.new().file_exists(path). It's very inconvenient.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
This method should be available statically, i.e. directly through the File class. There's no reason to have it as a member method. Other methods that could be made static:

  • Directory.file_exists()
  • Directory.file_exists()
  • Directory.make_dir/recursive()
  • Directory.remove()
  • Directory.rename()
    (each of these methods takes path to the file/directory, so calling it on an instance is unecessary)

As a bonus, File/Directory.open() could be static too and return an instance.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Make above-listed methods available statically, i.e. directly from File/Directory class, without needing to create an instance. Example class with similar functionality is Engine, where all methods are static, but you can create instance too (although it's invalid, heh)

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Well, literally one line, but it's about convenience. It is used often. Very often.

Is there a reason why this should be core and not an add-on in the asset library?:
Because core API would benefit from it, as it's a common thing to do.

@jonbonazza
Copy link

jonbonazza commented Jun 22, 2020

I think a better API would ve onr that is more Java-like.

The path is a property of the File object, so imo, should be declared in the constructor. I.e

var f := File.new(path)
var exists := f.exists()
if exists:
    var err := f.open()
    # ...

This is more ergonomic, more accurately adheres to OOP philosophies, and most importantly, is familiar to people coming from other popular programming languages.

@Calinou
Copy link
Member

Calinou commented Jun 22, 2020

To my knowledge, GDScript doesn't support exposing static methods yet (nonwithstanding singletons).

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 22, 2020

is familiar to people coming from other popular programming languages.

I'm coming from Ruby, where these methods are static ¯\_( •_•)_/¯

To my knowledge, GDScript doesn't support exposing static methods yet (nonwithstanding singletons).

The method has to be static only in GDScript. Also it should be possible to bind to non-static method that would call a static method.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 22, 2020

To my knowledge, GDScript doesn't support exposing static methods yet (nonwithstanding singletons).

Could actually solve an issue with ever-increasing number of singletons which have to be exposed for those kind of methods (at least that's my experience when writing some utility singletons via custom modules). Perhaps some singletons can also be "unexposed" which don't necessarily depend on the core functionality of Godot. JSON, Marshalls, Geometry: all have the purpose of utility/scoping of methods for convenience.

Lets make this happen:

void _File::_bind_methods() {
	ClassDB::bind_static_method(D_METHOD("file_exists", "path"), &_File::file_exists);
}

Note that the underlying FileAccess method is already declared static:

static bool exists(const String &p_name); ///< return true if a file exists

If using the same singleton-based approach ("singleton" might be too frightening name, because we're just interested in one instance of a particular class for the purpose of scoping methods), this needs a way of figuring out whether there are any methods which are exposed as static, and auto-instantiate and register a single instance of that class to be accessed in GDScript, if necessary.

@me2beats
Copy link

me2beats commented May 2, 2021

File.get_modified_time() could also be static I think

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 14, 2022

Closed by godotengine/godot#60408

@KoBeWi KoBeWi closed this as completed Oct 14, 2022
@KoBeWi KoBeWi added this to the 4.0 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Implemented
Development

No branches or pull requests

5 participants