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

Mono: Image.Load() doesn't recognize res:// outside of editor #20972

Closed
exts opened this issue Aug 13, 2018 · 21 comments · Fixed by #42412
Closed

Mono: Image.Load() doesn't recognize res:// outside of editor #20972

exts opened this issue Aug 13, 2018 · 21 comments · Fixed by #42412

Comments

@exts
Copy link
Contributor

exts commented Aug 13, 2018

Godot version:
3.0.6

OS/device including version:
Windows 10

Issue description:
Image.Load won't accepted res:// path's in exported projects, but works fine in editor. I was told image.load() wasn't meant for res:// paths (which doesn't make logical sense), so I'm assuming this method is broken or doesn't explicitly state its correct usage in the method name.

Steps to reproduce:

  1. Create an asset
  2. Try loading it using
                var image = new Image();
                image.Load($"res://pathtoasset");
  1. Create an image texture
                var imageTexture = new ImageTexture();
                imageTexture.CreateFromImage(image, (int) Texture.FlagsEnum.Mipmaps);
  1. Add it to the scene
  2. Run scene to make sure it outputs
  3. Export project and watch the image not print to the screen because it can't find the .png path

Minimal reproduction project:
Will create one after ludum dare 42

@FeralBytes
Copy link
Contributor

@exts shouldn't you be using an absolute path, not a resource path?

@exts
Copy link
Contributor Author

exts commented Aug 13, 2018

@FeralBytes yes, the above is a placeholder. the original code was something along the lines with:

var img = new Image();
img.Load($"{assetPath}/{food.Value}");

where $"{assetPath}/{food.Value}" looked something like: res://Assets/Art/Food/Meats/Chicken.png

Workaround was just calling it like so:

                var imageTexture = (Texture) GD.Load($"{assetPath}/{food.Value}");
                imageTexture.Flags = (int) Texture.FlagsEnum.Mipmaps;

which worked fine, i still don't see why image.load() doesn't work though with res:// paths,

@FeralBytes
Copy link
Contributor

@exts just to make sure we are on the same page. I am saying the path should not be res://Assets/Art/Food/Meats/Chicken.png but rather it should be an absolute path like /home/user/.godot/myGame/Downloaded/Assets/Art/Food/meats/Chicken.png. The path could be to any wheres but since you are using things outside of the executable then your path should not be res:// If the path is interal to the executable then the path should work fine, but you will have to handle 2 conditions when running in the editor use the standard res://Assets/Art/Food/Meats/Chicken.png but once exported to executable if you path is dynamically generated realize that the code will be only seeing res://Assets/Art/Food/Meats/Chicken.png.import.

@exts
Copy link
Contributor Author

exts commented Aug 13, 2018

that makes absolutely no sense lol why would you use absolute paths with an api library that targets multiple os's?

If I pass "res://" to any load method it should grab my local resources from the project root. Else the method needs to be renamed to something else

@FeralBytes
Copy link
Contributor

Your part about that making no sense is you mis understanding what I meant. as I state if you read the entire post, if the path is a resource within your project then res:// is fine but outside of your project you will need to use absolute paths. It seemed to me from what you wrote that you were actually trying to load an external file.

I think you need to read Data Paths.

Yes exactly it will grab your local resources from the project root. So lets do this as a test you try and load an asset using the path you provided res://Assets/Art/Food/Meats/Chicken.png, not dynamic and tell me if it works.
For reference the method is Image.load()

@exts
Copy link
Contributor Author

exts commented Aug 13, 2018

Your part about that making no sense is you mis understanding what I meant. as I state if you read the entire post, if the path is a resource within your project then res:// is fine but outside of your project you will need to use absolute paths. It seemed to me from what you wrote that you were actually trying to load an external file.

Not the case. I was only loading resources from within the project assets that were exported which have .stex files. Using Image.Load() does not load those assets, only can I load those assets using GD.Load()

Here you can see the code here https://github.com/exts/godotsharp-shortserve/blob/master/App/Food/FoodLoader.cs#L81 this is the updated code.

The original method was:

private static void CreateTextures(string type, Godot.Dictionary<string, string> foods, ICollection<FoodItem> foodItemTexturess, string assetPath)
{
    foreach(var food in foods)
    {
        var image = new Image();
        image.Load($"{assetPath}/{food.Value}");

        var imageTexture = new ImageTexture();
        imageTexture.CreateFromImage(image, (int) Texture.FlagsEnum.Mipmaps);

        foodItemTexturess.Add(new FoodItem(food.Key, type, imageTexture));
    }
}

It seemed to me from what you wrote that you were actually trying to load an external file.

You can try compiling that code and tell me why it shouldn't work with the original method because I'm not looking for an external file.

@PetePete1984
Copy link
Contributor

To clarify a bit, Image.Load() seems to expect a path to a physical file: it works when the file is present while you're running the project from the editor, but once you're into exported virtual filesystem .pck territory, Image.Load doesn't get redirected to the imported .stex files (the .png doesn't exist in the .pck at that point).
In contrast to that, GD.Load() explicitly tries to load a Resource and as such probably respects the virtual filesystem (ie it loads the .stex when you tell it to load the corresponding .png).
Absolute or relative paths are irrelevant to the problem.

@reduz
Copy link
Member

reduz commented Aug 13, 2018 via email

@exts
Copy link
Contributor Author

exts commented Aug 13, 2018

@reduz yeah that was the solution to fix my problem. Almost didn't make the ldjam when I ran into the issue 😄. Definitely confused me when I found out.

@PetePete1984
Copy link
Contributor

Update: Seems to also be the case for ImageTexture.load()

@reduz
Copy link
Member

reduz commented Aug 14, 2018 via email

@Zylann
Copy link
Contributor

Zylann commented Nov 18, 2018

Please don't remove Image.load, or you will piss off everyone loading non-resource images dynamically in their games and tools :/

@exts
Copy link
Contributor Author

exts commented Nov 18, 2018

or better yet just have proper support for res:// and user:// in exported games because currently it doesn't seem to work @Zylann

@Zylann
Copy link
Contributor

Zylann commented Nov 18, 2018

@exts does this work? Because it's the same deal^^

var f = File.new()
f.open("res://something.x", File.READ)

If Image.load is a confusion, renaming it Image.load_from_file would make more sense. And yeah, it should support the same kind of paths File accepts.

@exts
Copy link
Contributor Author

exts commented Nov 18, 2018

@Xrayez
Copy link
Contributor

Xrayez commented Nov 20, 2018

Please don't remove Image.load, or you will piss off everyone loading non-resource images dynamically in their games and tools :/

@Zylann yep, I really depend on this as it's one of the core features currently used in my game.

As I understand, loading a texture by path will return imported one (StreamTexture). What if it can be made that if the loader doesn't find imported texture by path, it should fallback to loading other compatible resource types? In this case it would be Image.

var img = load("my_image.png") # returns `StreamTexture` if found, else it's an `Image`

@KoBeWi
Copy link
Member

KoBeWi commented Jun 17, 2020

Can anyone still reproduce this bug in Godot 3.2.1 or any later release (e.g. 3.2.2-rc1)?

If yes, please ensure that an up-to-date Minimal Reproduction Project (MRP) is included in this report (a MRP is a zipped Godot project with the minimal elements necessary to reliably trigger the bug). You can upload ZIP files in an issue comment with a drag and drop.

@exts
Copy link
Contributor Author

exts commented Jun 17, 2020

@KoBeWi let me try reproducing it today.

Looks like this is intended then.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 17, 2020

@KoBeWi it seems like this is only a limitation when you try to image.load in exported games, which doesn't work, and should likely just be documented there (there's a warning for it in 3.2+).

The "opposite" use case is here though: #39396 (the warning should be moved to documentation IMO).

@KoBeWi
Copy link
Member

KoBeWi commented Jun 17, 2020

Tagging as documentation then.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 17, 2020

Looks like this is intended then.

I wouldn't say this is intended, but that's just a limitation that you have to be aware of. You could optionally try to include all image files while exporting, I would be actually interested if this works:

Annotation 2020-06-17 202156

But instead, if you need to load an image specifically, you just need to reimport the texture as an Image resource:

Annotation 2020-06-17 202452

If you want to distribute some images along with exported project you can also try globalizing the res:// path before doing so, so that the engine won't look into the virtual filesystem, but instead search for OS filesystem.

static func image_load(filepath):
    var image = Image.new()
    image.load(ProjectSettings.globalize_path(filepath))
    return image

@akien-mga akien-mga added this to the 4.0 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants