CreedVI / Raylib-J

Handmade Java binding for Raylib
zlib License
91 stars 15 forks source link

FileIO fails to load some files #61

Open DNAmaster10 opened 1 month ago

DNAmaster10 commented 1 month ago

Describe the bug FileIO fails to load some files.

To Reproduce Steps to reproduce the behaviour:

  1. Load any image from resources folder

  2. Image does not get loaded.

  3. Try to load shader file from a folder within the resources folder (Such as resources/shaders)

  4. Files will not load unless present in the root /resources folder.

Expected behaviour Files should load correctly

Code public static final Shader STAR_SHADER = Window.getWindow().core.LoadShader("shaders/star.vs", "shaders/star.rs");

Image image = rl.textures.LoadImage("image.png")

Desktop (please complete the following information): Operating System: KDE neon 6.0 KDE Plasma Version: 6.1.2 KDE Frameworks Version: 6.4.0 Qt Version: 6.7.0 Kernel Version: 6.5.0-44-generic (64-bit) Graphics Platform: Wayland Processors: 12 × AMD Ryzen 5 5600X 6-Core Processor Memory: 31.3 GiB of RAM Graphics Processor: AMD Radeon RX 6700 XT

Additional context When debugging, it seems that (At least for the image), FileIO is able to correctly load the information from the file, but is not able to parse that information into an Image object.

I'll look further into the issue when I have some time.

Thanks!

CreedVI commented 1 month ago

Would the file that fails to load be your fragment shader file? I see in your example code you have shaders/star.rs

I'm not sure if this is a typo on your issue or in your code, but .rs is not a shader file type recognised by raylib.

Consider changing the file type to .fs and see if that resolves the error.

DNAmaster10 commented 1 month ago

Hi. I've done some further testing with the shaders specifically.

First, I tried just setting the file extension to being a random word, so I had "fragment.test" & "vertex.foobar". Interestingly, with the following file structure, this seemed to work fine. image

I loaded these shaders with: Shader shader = rl.core.LoadShader("vertex.foobar", "fragment.test");

Next, I tried moving them into the "test" folder, and renamed them to have the correct file extensions. image

I loaded these shaders with: Shader shader = rl.core.LoadShader("test/vertex.vs", "test/fragment.fs");

This produced an error:

Exception in thread "main" java.lang.NullPointerException
    at java.base/java.io.Reader.<init>(Reader.java:168)
    at java.base/java.io.InputStreamReader.<init>(InputStreamReader.java:88)
    at com.raylib.java.utils.FileIO.LoadFileText(FileIO.java:124)
    at com.raylib.java.core.rCore.LoadShader(rCore.java:1384)
    at org.dnamaster10.Main.main(Main.java:23)

I tried other variations of LoadShader, with file paths starting with a "/", etc, but this seemed to throw the same error.

Checking the generated class files, it seems the directory structure has been preserved, and the shader files are still located in the "test" directory within the resource folder.

Looking into this further, it appears the NullPointerException is thrown in the FileIO class at line 122. https://github.com/CreedVI/Raylib-J/blob/main/src/main/java/com/raylib/java/utils/FileIO.java

Viewing through my debugger, it appears that inputStream is null by the time it tries to read the file.

I might be completely wrong in my assessment here, but I think that if the else statement at line 117 was run, it would be able to find the file at the specified path.

bramtechs commented 2 weeks ago

The following code fixes the four reproduction steps for me.

/**
     * Load text data from file
     * NOTE: text chars array should be freed manually
     *
     * @param fileName name and extension of file to be loaded
     */
    public static String LoadFileText(String fileName) throws IOException{
        if (fileName != null){

            if (SUPPORT_STANDARD_FILEIO){
                InputStream inputStream = FileIO.class.getResourceAsStream("/"+fileName);      //  <--- changed
                if(inputStream == null) {
                    String ext = fileName.substring(fileName.lastIndexOf('.')).toUpperCase();
                    inputStream = getFileFromResourceAsStream(fileName.substring(0, fileName.lastIndexOf('.'))+ext);
                }
                BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));

                String[] tmp = reader.lines().toArray(String[]::new);
                StringBuilder builder = new StringBuilder();

                for (String s : tmp) {
                    builder.append(s);
                    builder.append("\n");
                }

                if (tmp != null) {
                    return builder.toString();
                }
                else{
                    Tracelog(LOG_WARNING, "FILEIO: [" + fileName + "] Failed to open text file");
                }
            }
            else{
                Tracelog(LOG_WARNING, "FILEIO: Standard file io not supported, use custom file callback");
            }
        }
        return null;
    }

I haven't tested this with the examples yet, but the following if-else branch in the current implementation seems redundant to me. What was the reason for this lastIndexOf and if-else?

InputStream inputStream;
if (fileName.contains("/")) {
    inputStream = FileIO.class.getResourceAsStream(fileName.substring(fileName.lastIndexOf('/')));
}
else {
    inputStream = FileIO.class.getResourceAsStream("/"+fileName);
}
image
bramtechs commented 2 weeks ago

Using paths without prefixed slash also work.

image