openfl / lime

A foundational Haxe framework for cross-platform development
https://lime.openfl.org/
MIT License
749 stars 359 forks source link

Stop shadowing `haxe.io.Bytes`. #1765

Open player-03 opened 4 months ago

player-03 commented 4 months ago

Lime's version of the class has fallen behind Haxe's over time, and I believe it's past time to return to using their version. Shadowing a core class should always be a last resort.

Lime does add a few things to the class, so we want to make sure we can still get the same functionality.

I checked line by line in a diff editor to make sure that was everything. All other differences either had no functional effect (whitespace and so on), or were because Haxe's version was newer.

joshtynjala commented 4 months ago

Shadowing a core class should always be a last resort.

Agreed. I'd even go as far as saying it should never be done. It's one thing to shadow another library's class, if you know the risks and there's seemingly no other choice. It's a giant can of Cthulhus to shadow a standard library class.

joshtynjala commented 4 months ago

[C++] Lime replaces #if cpp with #if (cpp || webassembly). As far as I know there's no way to change #if statements without shadowing the file, so I set it up to do so specifically on WebAssembly. That version may fall behind over time, but at least everything else won't.

I may be showing my ignorance here, but doesn't webassembly support come from hxcpp? I'm surprised that the cpp flag isn't set when targeting wasm.

player-03 commented 4 months ago

I may be showing my ignorance here, but doesn't webassembly support come from hxcpp? I'm surprised that the cpp flag isn't set when targeting wasm.

I've never used Wasm, so I wouldn't know. I prefer to only make changes that I fully understand. But if we can test it and show that the cpp flag is set, then it should be safe to remove. (Currently looking up how to install the Emscripten SDK.)

player-03 commented 4 months ago

I tested, and the flag is indeed set. That makes things much simpler!

And it makes sense. Why would C++ functionality like the blit() function be available if the cpp flag wasn't set?

player-03 commented 3 months ago

Update: this must be merged by the time Haxe 5 releases.