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

Add notes on DirAccess documentation #99132

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Add notes on DirAccess documentation #99132

merged 1 commit into from
Nov 14, 2024

Conversation

BrianBHuynh
Copy link
Contributor

Some notes are ported from FileAccess (for example file_exist) Other notes were added when needed (for example when included on the non static version but not on the static version) Other entirely new notes were added as well when required for example when getting a list of directories or if a directory exist or not

Clarified note at the top and made it more in line with the one found in file access

@BrianBHuynh BrianBHuynh requested a review from a team as a code owner November 12, 2024 16:05
@Calinou Calinou added enhancement documentation cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Nov 12, 2024
@Calinou Calinou added this to the 4.4 milestone Nov 12, 2024
@BrianBHuynh
Copy link
Contributor Author

Some of the notes are because of #99047 and similar issues. As shown in the minimum reproduction project the directories can just not exist inside the .pck file in some edge cases, and at the same time (like for FileAccess) files may not be included as well! These clarifications should make things more clear while in the editor, since although some were just copy and pasted from elsewhere, it will be easier for people using the engine to debug.

@BrianBHuynh
Copy link
Contributor Author

Holy- you're fast LOL I was typing an addendum and saw you add the tags while I was about to press comment @>@ thanks for the review @Calinou

@AThousandShips AThousandShips changed the title Added notes on DirAccess Add notes on DirAccess documentation Nov 12, 2024
@BrianBHuynh
Copy link
Contributor Author

:D radiant also helped me with some formating on the discord as well! Wanted to mention it since I'm not too familiar with adding them as a contributor on github

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Not much of a fan of the sheer verbosity of these notes, but this information is important and it's better than not having it.

I could try to come up with different notes outright, but instead I'll just suggest tweaks in and there to mitigate this.

@BrianBHuynh
Copy link
Contributor Author

O7 alright, I thought it'd be better to be verbose than not since it's confusing when something works in the editor but not outside of it (as compared to something that you can test in the editor)

@BrianBHuynh
Copy link
Contributor Author

Just accepted and squashed most of those changes (I asked around just now and also personally think differ sounds better in this context than be different so that was the only one I didn't accept) @Mickeon

@BrianBHuynh
Copy link
Contributor Author

There we go :D all neat and consistent now ✨

Some notes are ported from FileAccess (for example file_exist)
Other notes were added when needed (for example when included on the non static version but not on the static version)
Other entirely new notes were added as well when required for example when getting a list of directories or if a directory exist or not

Clarified note at the top and made it more in line with the one found in file access

Co-Authored-By: Micky <66727710+Mickeon@users.noreply.github.com>
@Repiteo Repiteo merged commit 673f396 into godotengine:master Nov 14, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 14, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants