Skip to content

Conversation

@inc0der
Copy link
Contributor

@inc0der inc0der commented Aug 14, 2023

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 jeremyfa merged commit 7378099 into ceramic-engine:master Aug 19, 2023
@jeremyfa
Copy link
Member

jeremyfa commented Aug 19, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants