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

[FileAccess] Add methods to get/set "hidden" and "read-only" attributes on macOS/BSD and Windows. #80404

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Aug 8, 2023

Adds (get)set_read_only_attribute and (get)set_hidden_attribute methods, and exposes them and methods to get/set UNIX permissions to the scripting API.

Useful for making temporary files hidden on Windows, e.g. #80188 (review)

@Calinou
Copy link
Member

Calinou commented Aug 8, 2023

This should also make it possible to implement godotengine/godot-proposals#801 if desired (though personally, I wouldn't do that for cross-platform consistency reasons).

<param index="0" name="file" type="String" />
<param index="1" name="permissions" type="int" />
<description>
Sets file UNIX permissions.
Copy link
Member

@akien-mga akien-mga Aug 8, 2023

Choose a reason for hiding this comment

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

Might be good to document what kind of values [param permissions] support (probably not the full blown description of possible permission values, but e.g. is it supposed to take 0x755 for the equivalent to chmod 755?).

Copy link
Member Author

Choose a reason for hiding this comment

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

chmod 755 is octal, so it takes0x1ED (GDScript do not have octal support). Added a list of values to combine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we define an enum for these values then? The octal format is widespread and so it's easy to read with some experience, and this is less so. But maybe it's an overkill.

Copy link
Member

Choose a reason for hiding this comment

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

An enum would be good for usability, but I think this might be a niche enough feature that it may not be worth it to add more cognitive overload in the FileAccess API by adding an enum with all these values.

I wouldn't oppose it if @bruvzg thinks it's worth adding, but we can also just wait to see if there's demand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it to bitfield enum.

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Seems to work as expected on Windows.

One usability note: we could probably improve this error message now, when trying to write a readonly file. Or rather add an extra check before reaching this error and report the actual state of the file.

Safe save failed. This may be a permissions problem, but also may happen because you are running a paranoid antivirus. If this is the case, please switch to Windows Defender or disable the 'safe save' option in editor settings. This makes it work, but increases the risk of file corruption in a crash.

It's in FileAccessWindows::_close().

@bruvzg bruvzg force-pushed the file_attribs branch 2 times, most recently from 2954eee to f5ae23c Compare August 8, 2023 18:30
@akien-mga akien-mga modified the milestones: 4.x, 4.2 Aug 9, 2023
@akien-mga akien-mga merged commit d255811 into godotengine:master Aug 9, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

4 participants