jaylib icon indicating copy to clipboard operation
jaylib copied to clipboard

LoadFileText method not working as intended

Open redfox4ever opened this issue 1 year ago • 7 comments

When I call LoadFileText it returns a ByteBuffer with the capacity of 1 regardless of the content of the file loaded.

package com.redfox.platformer;

import java.nio.ByteBuffer;

import static com.raylib.Colors.RAYWHITE;
import com.raylib.Raylib;
import static com.raylib.Raylib.BeginDrawing;
import static com.raylib.Raylib.ClearBackground;
import static com.raylib.Raylib.CloseWindow;
import static com.raylib.Raylib.EndDrawing;
import static com.raylib.Raylib.InitWindow;
import static com.raylib.Raylib.WindowShouldClose;

public class Game {
    public static void main(String[] args) {
        int screenWidth = 800;
        int screenHeight = 600;
        InitWindow(screenWidth, screenHeight, "raygui");
        while (!WindowShouldClose()) {
            BeginDrawing();
            ClearBackground(RAYWHITE);
            EndDrawing();
        }
        CloseWindow();
        ByteBuffer text = Raylib.LoadFileText("resources\\Text.txt");
        System.out.println(text.capacity());  // 1
        while(text.hasRemaining())
        {
            System.out.printf("%c", (char)text.get());
        }   
        System.out.println(); 
    }
    
}

resources/Text.txt:

esfsdfsdfsdfs
sdfsdfds

redfox4ever avatar Dec 26 '24 11:12 redfox4ever

This is probably a bug in JavaCPP. I will investigate to try to be sure.

Fortunately there is a work-around: use Java standard library instead of Raylib for loading text. All the Raylib text manipulation functions are only there because C lacks them so they shouldn't really be used in languages like Java.

electronstudio avatar Dec 26 '24 15:12 electronstudio

The code generated by JavaCPP:

JNIEXPORT jobject JNICALL Java_com_raylib_Raylib_LoadFileText__Ljava_lang_String_2(JNIEnv* env, jclass cls, jstring arg0) {
    const char* ptr0 = JavaCPP_getStringBytes(env, arg0);
    jobject rarg = NULL;
    char* rptr;
    jthrowable exc = NULL;
    try {
        rptr = (char*)LoadFileText(ptr0);
        jlong rcapacity = rptr != NULL ? 1 : 0;
        if (rptr != NULL) {
            jlong rcapacityptr = rcapacity * sizeof(rptr[0]);
            rarg = env->NewDirectByteBuffer((void*)rptr, rcapacityptr < INT_MAX ? rcapacityptr : INT_MAX);
        }
    } catch (...) {
        exc = JavaCPP_handleException(env, 8);
    }

    JavaCPP_releaseStringBytes(env, arg0, ptr0);
    if (exc != NULL) {
        env->Throw(exc);
    }
    return rarg;
}

So problem is rcapacity is always set to 1.

This is kind of the C pointer decay issue. There is no way of knowing if Raylib has returned a pointer to a single char, or to an array of chars. And if it is an array, the only way of knowing what rcapacity should be is to scan it for for the null terminator at the end. It's all pretty unsafe.

However JavaCPP manages to handle other functions that return char arrays OK... it does

 rarg = JavaCPP_createStringFromBytes(env, rptr);

Seems it knows to do that because the others have return type const char* not char*

So I guess probably not reasonable to expect JavaCPP to handle Raylib having a shitty API? We'll just have to modify raylib API to return const I think?

electronstudio avatar Dec 26 '24 16:12 electronstudio

Or probably easier: we could just remove this function, since it's not the best way to read text anyway?

electronstudio avatar Dec 26 '24 16:12 electronstudio

There are 5 functions in the API that have this same issue, so we should fix or remove them all. I worry that if we fix them then there may be other text manipulation functions that also don't work properly, because it's generally a bad idea to use them, and I don't have time to test them all. So I'm thinking either remove all the text functions, or mark them as deprecated just in case someone does have a valid use for them.

electronstudio avatar Dec 26 '24 16:12 electronstudio

Adding deprecated annotation to functions in JavaCPP doesn't seem to work - it just deletes the function. The only examples I can find of adding annotations seem to be for fields not functions. So we would have to patch the generated java files to do this, which is annoying.

Modifying function signatures to fix them in JavaCPP doesn't seem to be possible either. Possibly I just haven't looked hard enough. So we have to patch the .h files with fixed signatures, which is annoying

The only thing that is easy to do in JavaCPP is to skip lines in .h file entirely to remove functions but that's my least prefered option.

electronstudio avatar Dec 26 '24 20:12 electronstudio

The function's here, because in C reading a file to an array is painful (I experienced that myself), but happily in Java it's easy, so yes, this function is useless, and should be removed.

glowiak avatar Dec 28 '24 20:12 glowiak

Have fixed it to return a string and also marked it as deprecated. But it required manually copying the whole function sig so isn't a good way of handling it really. Need to automate it to mark all the text functions deprecated. Maybe a script that searches for any function named 'Text*' and inserts @deprecated

electronstudio avatar Dec 31 '24 18:12 electronstudio