rhaiscript / rhai

Rhai - An embedded scripting language for Rust.
https://crates.io/crates/rhai
Apache License 2.0
3.79k stars 177 forks source link

Handle f32/f64 difference more gracefully or explicitly state how to manage them #274

Closed dmadisetti closed 3 years ago

dmadisetti commented 3 years ago

Hello again,

I can pass down a f32 through a serded object, but I then can't use that value since all floats are by default f64, and the machinery to easily type cast or do anything with f32 (or the documentation for this), is missing- I suppose it's as easy as registering the arithmetic operations or adding f32 to to_float. As is: Function not found '* (f32, f64)'. I see there's a global FLOAT, but I just want to multiple values, not change the whole project, and it looks immutable anyway https://github.com/jonathandturner/rhai/blob/edd136c04755de9766e8d36cc14494e158300898/src/parser.rs#L66

Let me know your thoughts

schungx commented 3 years ago

Yes, there is no to_float from f32 to f64... Never thought it would be useful. I can add one...

However, f32 is currently handled as a custom type, meaning that Rhai doesn't know about it by default, and you have to register your own API on it.

There is built-in support for f64 in Dynamic, that's why serde will always convert f32 and f64 into a Dynamic with that float. If you want f32 to parse into a f32 custom type for Rhai, you'll have to override the default serializer...

Understanding that there is only f64 supported in Dynamic and f32 is treated as any custom Rust type, do you have any suggestion on a graceful way to treat it?

dmadisetti commented 3 years ago

Hmm, but there's obviously partial support for f32s, they're not totally custom. For instance, the API lets me add f32s... This is a pretty weird space for f32s to be.

What I think I was getting at is doing something similar to INT https://github.com/jonathandturner/rhai/blob/edd136c04755de9766e8d36cc14494e158300898/src/parser.rs#L51-L67

So like:

/// The system floating type.
///
/// Not available under the `no_float` feature.
/// If the `only_f32` feature is not enabled, this will be `f64` instead.
#[cfg(not(feature = "no_float"))]
#[cfg(not(feature = "only_f32"))]
pub type FLOAT = f64;

#[cfg(not(feature = "no_float"))]
#[cfg(feature = "only_f32")]
pub type FLOAT = f32;

I am passing down an external library struct, I could either register a function to handle these floats or recreate the library struct and change all 32s to 64s- but that seems sticky for something so primitive. Setting only_f32, means I'm fine as long as I'm consistent.

Obviously first class handling and implicit casting would be best, why restrict dynamic to only whatever's set as FLOAT? What's stopping the extension of Union?

Alternatively, some way to convert would be perfect. One direction means that no data loss can occur, so is acceptable. Like f32s are pretty fundamental right?

schungx commented 3 years ago

There are two choices.

1) As you propose, add f32_float feature to map FLOAT to f32 instead of f64. This should be easy.

2) Add a new data type to Dynamic which is Union::Float32, making f32 support native in addition to f64 so they both co-exist in script. This is more involved.

No.2 also has the dilemma of making script confusing - which float am I using? How do I make an f32 float instead of f64? And why is my x + y not working? (because x is f32 and y is f64...)

schungx commented 3 years ago

OK, it seems that adding a new feature f32_float is the best solution; in that case, f32 is FLOAT and f64 is treated as a custom type.

This new feature will land in a future PR once I test it further...

schungx commented 3 years ago

f32_float is now alive with https://github.com/jonathandturner/rhai/pull/278

dmadisetti commented 3 years ago

Cheers! I'll give it a spin either later today or tomorrow