Replace File/Directory with FileAccess/DirAccess

[KoBeWi]

This PR removes Fille/Directory from core bind and binds FileAccess/DirAccess instead.

The "new" classes have almost the same API as the old ones, but there are some changes. Most notable changes are open() being static and lack of close() method. I updated the docs to reflect that.

[bruvzg]

Note that for now there is no error handling for the static methods.

I guess we can just add thread local get_last_error() method, returning Array won't be too intuitive for the users, and the error codes aren't too verbose to be useful.

[reduz]

For the error, I think you could do something like
thread_local Error last_file_open_error = OK in the cpp
then set it when calling the static open() function, finally a
static Error FileAccess::_get_open_error(); that you bind to the script to get that error

[raulsntos]

The include_navigational and include_hidden properties are now ignored because they used to be handled by the non-virtual Directory::get_next method. This causes an infinite loop in converter3to4 which seems to be what's making the CI fail.

[KoBeWi]

I totally forgot about get_next(). I added a _get_next() method for the binding to properly use the include properties.

[reduz]

@KoBeWi thread_local is useful, but it takes up stack from all threads, so it must be used with care.

[akien-mga]

Shouldn't those be virtual instead of abstract?

They can be instantiated so (pure) abstract doesn't seem correct to me.

[KoBeWi]

I don't know what's the difference. Does virtual also prevent manual instantiation?
I checked how we use abstract and TreeItem is abstract, but you can instantiate it too. Same with SceneTreeTimer.

[akien-mga]

Ah so we want to forbid FileAccess.new() to force using a create method?

Then abstract might work indeed, but it feels to me like it's exploiting the wrong concept to expose a class which is not abstract nor virtual but just means to be non-instantiable directly.

[reduz]

Yes, abstract is good in this case, if we want to provide something towards the future, in this case it will work out better as FileAccessExtension

[akien-mga]

Looks good to me.

I discussed with @reduz, @KoBeWi and @Faless the fact that this breaks compatibility significantly with 3.x / previous 4.0 builds as File and Directory in the scripting API will now be exposed as FileAccess and DirAccess (like they werehe already internally in C++ code).

The compat breakage is of course unfortunate, but we agreed that it's better as:

• It's the internal name and it's more consistent (those objects don't represent files or directories themselves, but the access to them).
• Even without that name change, this PR breaks compatibility as Directory.open() and File.open() are now static methods, so users will need to fix their code anyway and this makes it more obvious than a potential silent failure from misusing the new API.

[akien-mga]

Thanks!

[CsloudX]

IMO, I think File and Directory was a better name than FileAccess and DirAccess, I don't know why to make this change, is where a disccuss?

old	new
var file = File.new()	var ??? = FileAcess.new() # file, file_access, access! which name better?

And for another reason, in C#, Java, Qt, access file class's name all was File, why Godot named FileAccess?
So I wich it will be named back to File and Directory please.

OTHER: make such method like get_md5 to static was a good thing, I like that.

[KoBeWi]

The new is actually FileAccess.open(), you can no longer create non-opened file/directory, thus the usage has changed.

The old File/Directory were wrappers that used FileAccess/DirAccess under the hood. This PR just strips the wrapping layer and exposes FileAccess/DirAccess directly; the "new" classes just use the core name (changing it is a big deal, because it's used ~2700 times in the engine code, so that would be a lot of changes). Also the new names are more accurate, because you are not creating "File" or "Directory", you are just accessing them from the filesystem. And as I mentioned, the new classes are used a bit differently (no more open/close), so new names signify that.

[dmaz]

I don't want to complain but I don't understand the need for a change like this. Godot is supposed to be in beta now yet this is a pretty significant breaking change. there is a lot of script that will need to change with this.. it's not just a rename. the change also touched a lot of src files which of course invalidates all any previous testing during the alpha stage for those. this breaks style with other classes like ConfigFile. and with regards to "names are more accurate", who thought that's the case in the first place. using new() is perfectly consistent with how all classes work in GDScirpt... this new form with the auto new is really not.

Now if this was needed for something that doesn't seem to be described here, well fine. I think though then that these PRs should include a little blurb explaining the reasoning for the change.

[KoBeWi]

The reason why this PR was created is that the previous Directory/File classes were a workaround for lack of static method bind support in GDScript. This is now possible, so we can get rid of most of the core bind file (which, as you can see in the changes here, just duplicates the methods of original classes and forwards them to a new class). The FileAccess/DirAccess are the only classes that required compatibility breaking, so they were the first ones to be replaced (unfortunately this PR didn't make it before beta1).

This is a breaking change, but not something that can't be fixed easily. Replacing it in my (rather big) project took me ~20 minutes. The API is mostly the same, the only thing that changed is how you create the objects. Also some of the methods are now static, so you can get rid of code like File.new().file_exists(path) (which in my case was 90% of the File instances).

[dmaz]

Ah! ok, thanks for the reply and I appreciate the explanation. totally get it now.

[TokisanGames]

The documentation hasn't been updated.

DirAccess says to use new() which doesn't work.

# Standard
var dir = Directory.new()
dir.open("user://levels")
dir.make_dir("world1")
# Static
Directory.make_dir_absolute("user://levels/world1")

[nikitalita]

when this was renamed to open_internal(), the _open() call below was left in place, which ends up calling FileAccess::_open(), which becomes an infinite recursion and causes a stack overflow.

[akien-mga]

Fixed by #66485.

@KoBeWi This would have been caught if _open was made private, which I think it could be. I see a few more underscore-prefixed methods public in DirAccess and FileAccess, I would suggest doing another pass on those to see what needs to be public and what can be made private or protected.