ceramic-engine / ceramic

Cross-platform 2D framework written in Haxe that can export natively to desktop (windows, mac, linux), mobile (ios, android), web (js + webgl) and to unity projects
MIT License
254 stars 22 forks source link

Fix Files.getBytes on web target #120

Closed inc0der closed 1 year ago

inc0der commented 1 year ago

Problem

There is a problem with Files.getBytes on web targets, because fs.readFileSync when using the encoding option of binary returns a string.

The compiled code:

return new haxe_io_Bytes(new Uint8Array(data.buffer));

is attempting to access data.buffer but because data is a string, buffer is undefined, this caused the new Uint8Array(data.buffer) to return an empty array in turn causing the bytes returned to simply be empty.

The test example

After creating a new ceramic project with ceramic init in the create method or preload method attempt to get bytes from a file on your system.

var bytes = ceramic.Files.getBytes('E:\\Projects\\haxe\\ceramic-scene-test\\assets\\ceramic.png');
trace(bytes);
// you can also attempt to load texture from bytes and it will throw an error 
Texture.fromBytes(bytes, (texture) -> {
  if (texture != null) {
    trace(texture);
  } else {
     trace('error loading from bytes');
  }
});

in this case bytes will be haxe_io_Bytes with a length of 0 with an empty UInt8array and attempting to get the texture from bytes results in failed to load image from bytes, on error: [object Event] image

Solution

The simplest solution would be to not use the binary encoding option from Files.getBytes method, as readFileSync will set encoding to null and will return a Buffer instead of a string and data.buffer will exist and the array and bytes will no longer be empty.

The solution:

var data:UInt8Array = fs.readFileSync(path);

I don't believe this will cause any unintended side effects, I have tested the solution in my own projects as well as a test project and there appears to be no further issues.

jeremyfa commented 1 year ago

The change looks good to me, surprising the 'binary' wasn't there already, but I guess that was an oversight from me :D

Thanks for the fix!