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

Support uid:// in more places #99286

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Nov 15, 2024

image
Adds uid:// support for Image, FileAccess and ProjectSettings.

Fixes #99221
Supersedes #99278

@KoBeWi KoBeWi added this to the 4.4 milestone Nov 15, 2024
@KoBeWi KoBeWi requested a review from a team as a code owner November 15, 2024 15:16
Copy link
Member

@syntaxerror247 syntaxerror247 left a comment

Choose a reason for hiding this comment

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

This resolves my issue and seems to be a better solution, as it eliminates the need for manual conversions everywhere.

Copy link
Member

@reduz reduz left a comment

Choose a reason for hiding this comment

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

I guess at the end of the day, best solution is simplest solution

@syntaxerror247
Copy link
Member

syntaxerror247 commented Nov 16, 2024

I got another issue now. It works in editor on PC but boot splash loading fails when I run my app on android.

image

core/io/file_access.cpp Outdated Show resolved Hide resolved
Copy link
Member

@syntaxerror247 syntaxerror247 left a comment

Choose a reason for hiding this comment

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

In addition to the issue mentioned in my above comment, I'm also not able to create a gradle build for android. During export, I am getting these errors 👇
image

@KoBeWi KoBeWi force-pushed the uid_in_a_path_factory branch from 125cde8 to 4da8f49 Compare November 16, 2024 18:51
@KoBeWi KoBeWi requested a review from a team as a code owner November 16, 2024 18:51
@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 16, 2024

@syntaxerror247 Please check again.

@syntaxerror247
Copy link
Member

@KoBeWi Still same error with gradle export. Boot splash doesn't throw error now but icon is still not visible.

@KoBeWi KoBeWi force-pushed the uid_in_a_path_factory branch from 4da8f49 to 3b6705a Compare November 16, 2024 20:43
@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 16, 2024

Boot splash doesn't throw error now but icon is still not visible.

Eh, I'm not sure if it's related to UIDs anymore. The error you posted clearly says that res:// file is missing.

I can't locate where the gradle error comes from.

@syntaxerror247
Copy link
Member

syntaxerror247 commented Nov 17, 2024

Eh, I'm not sure if it's related to UIDs anymore. The error you posted clearly says that res:// file is missing.

I’m still confident this issue is related to UIDs. Today, I compiled the engine with and without your PR. The boot splash error occurs in both variants, but in the latter, I can workaround it by setting the boot splash logo using 4.4 dev4 and then export with my custom build. This strongly suggests that the issue is related to UIDs.

Regarding the Gradle build issue: without this PR, I encounter problems with setting adaptive icons, but at least the build completes. However, with this PR, the build doesn’t even start and fails immediately.
I think I have an idea of where gradle build error might be coming from. I’ll investigate further on this one.

@syntaxerror247
Copy link
Member

@KoBeWi I've fixed both issues in my fork, building on your work. I'll create a PR to supersede this one.

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 17, 2024

@syntaxerror247 I don't see why your PR would supersede mine, just remove the changes except Android ones. This PR was already reviewed and approved, so it can be merged right now.

@syntaxerror247
Copy link
Member

syntaxerror247 commented Nov 17, 2024

@KoBeWi I wasn't sure what to do, would it be possible to include my changes in this PR?

EDIT: I have updated my PR to only include fix for android issues.

@Repiteo Repiteo merged commit 0dda6a9 into godotengine:master Nov 18, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 18, 2024

Thanks!

@KoBeWi KoBeWi deleted the uid_in_a_path_factory branch November 18, 2024 15:31
@olivatooo
Copy link

is there a way to remove those uids? They are horrible for git based projects

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 26, 2024

No.

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.

Image loading is broken in editor
6 participants