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

Sync with Godot master #53

Merged
merged 13 commits into from
May 26, 2022
Merged

Sync with Godot master #53

merged 13 commits into from
May 26, 2022

Conversation

lufog
Copy link
Contributor

@lufog lufog commented May 25, 2022

@lufog
Copy link
Contributor Author

lufog commented May 25, 2022

Hooray! The module is compiles and works with Godot master branch. From unresolved:

  • I have no idea what to do with PVRTC.
  • Most likely I broke a lot of things during 49e0449, but I'm tired, and I'll deal with it later.

@lufog lufog marked this pull request as ready for review May 25, 2022 19:09
@lufog lufog changed the title [WIP] Sync with Godot master Sync with Godot master May 25, 2022
the Image::Format enum was changed, so we have to use a compatibility function to convert to the correct enum value
Because we don't explicitly close or delete files any more, file handles will remain open if referenced and have to be explicitly unreferenced to close them if that reference does not go out of scope

We also have to explicitly flush or unreference the object in order to get it to write the bytes to disk
@nikitalita
Copy link
Collaborator

Thank you very much for this! I was really dreading updating the master sync for release, so I'm glad someone did it for me :)

I added a few commits:

  • the file format extension for v4 textures changed, so I added those extensions to the relevant filters
  • the removal of PVRTC support means that we can no longer open v2 or v3 textures in that format; It doesn't appear to have widely been used, so I just removed support.
    • Irritatingly, this means that the Image::Format enum changes between v3 and v4, so I had to create a compatibility function to convert v3 image format numbers to the correct enumeration value and
  • FileAccess changing to a ref-counted object introduces unintended consequences. We have to make doubly sure that:
    • The fileaccess actually goes out of scope when we are done reading/writing. If the reference hangs around, that means the file will remain open forever, so we have to explicitly unreference it.
    • If we need to read a file right after writing it, we have to explicitly call flush() or make sure that the fileaccess object is actually unreferenced, or unwritten bytes will hang around in the buffer and not be written to disk
  • Every time we sync with master, we have to make sure to change the sync reference commit in the CI scripts, or the CI builds will fail

It's surprising to me just how much the API is changing at this late in the development cycle; I probably won't update the sync again until v4 gets closer to release, I think.

@nikitalita
Copy link
Collaborator

CI is currently broken, but I've validated this locally. I'll PR a fix for the CI shortly.

@nikitalita nikitalita merged commit 2e2669a into GDRETools:master May 26, 2022
@lufog lufog deleted the sync_with_master branch May 27, 2022 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants