-
-
Notifications
You must be signed in to change notification settings - Fork 922
avm2: Implement Math methods in Number #21018
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use crate::avm2::error::type_error; | |
use crate::avm2::object::Object; | ||
use crate::avm2::value::Value; | ||
use crate::avm2::{ClassObject, Error}; | ||
use num_traits::ToPrimitive; | ||
use rand::Rng; | ||
|
||
macro_rules! wrap_std { | ||
|
@@ -20,14 +21,11 @@ macro_rules! wrap_std { | |
}; | ||
} | ||
|
||
wrap_std!(abs, f64::abs); | ||
wrap_std!(acos, f64::acos); | ||
wrap_std!(asin, f64::asin); | ||
wrap_std!(atan, f64::atan); | ||
wrap_std!(ceil, f64::ceil); | ||
wrap_std!(cos, f64::cos); | ||
wrap_std!(exp, f64::exp); | ||
wrap_std!(floor, f64::floor); | ||
wrap_std!(log, f64::ln); | ||
wrap_std!(sin, f64::sin); | ||
wrap_std!(sqrt, f64::sqrt); | ||
|
@@ -66,7 +64,10 @@ pub fn round<'gc>( | |
// Note that Flash Math.round always rounds toward infinity, | ||
// unlike Rust f32::round which rounds away from zero. | ||
let ret = (x + 0.5).floor(); | ||
Ok(ret.into()) | ||
match ret.to_i32() { | ||
Some(num) => Ok(Value::Integer(num)), | ||
None => Ok(Value::Number(ret)), | ||
} | ||
} | ||
|
||
pub fn atan2<'gc>( | ||
|
@@ -90,7 +91,9 @@ pub fn max<'gc>( | |
let val = arg.coerce_to_number(activation)?; | ||
if val.is_nan() { | ||
return Ok(f64::NAN.into()); | ||
} else if val > cur_max { | ||
} else if val > cur_max | ||
|| (val == cur_max && !val.is_sign_negative() && cur_max.is_sign_negative()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's an oddly specific condition. Why does it have to be here? Can't it be somehow generalized? (Similarly the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It handles signed zero cases. In max, positive zero takes priority, while in min, the negative zero takes priority. Rust cannot compare positive zero with negative zero, so that's why it is needed. |
||
{ | ||
cur_max = val; | ||
}; | ||
} | ||
|
@@ -107,7 +110,9 @@ pub fn min<'gc>( | |
let val = arg.coerce_to_number(activation)?; | ||
if val.is_nan() { | ||
return Ok(f64::NAN.into()); | ||
} else if val < cur_min { | ||
} else if val < cur_min | ||
|| (val == cur_min && val.is_sign_negative() && !cur_min.is_sign_negative()) | ||
{ | ||
cur_min = val; | ||
} | ||
} | ||
|
@@ -121,8 +126,16 @@ pub fn pow<'gc>( | |
) -> Result<Value<'gc>, Error<'gc>> { | ||
let n = args[0].as_f64(); | ||
let p = args[1].as_f64(); | ||
|
||
Ok(f64::powf(n, p).into()) | ||
match (n, p) { | ||
(_, _) if p.is_nan() => Ok(f64::NAN.into()), | ||
// Special case: If p is ±Infinity and n is ±1, the result is NaN. | ||
(1.0, _) | (-1.0, _) if p.is_infinite() => Ok(f64::NAN.into()), | ||
// Special case: If n is -Infinity and p < 0 and p is a negative even integer, Flash Player returns -0. | ||
(f64::NEG_INFINITY, _) if p.to_i64().is_some_and(|i| i % 2 == 0 && i < 0) => { | ||
Ok(Value::Number(-0.0)) | ||
} | ||
(_, _) => Ok(f64::powf(n, p).into()), | ||
} | ||
} | ||
|
||
pub fn random<'gc>( | ||
|
@@ -136,3 +149,64 @@ pub fn random<'gc>( | |
let rand = activation.context.rng.random_range(0..MAX_VAL); | ||
Ok(((rand as f64) / (MAX_VAL as f64 + 1f64)).into()) | ||
} | ||
|
||
pub fn abs<'gc>( | ||
_activation: &mut Activation<'_, 'gc>, | ||
_this: Value<'gc>, | ||
args: &[Value<'gc>], | ||
) -> Result<Value<'gc>, Error<'gc>> { | ||
let input = args[0]; | ||
match input { | ||
Value::Integer(i) => Ok(i.abs().into()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have dedicated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a pushbyte instruction in the test from_avmplus/as3/Types/Number/abs, which (if I understand it correctly) pushes a byte (and therefore an integer) to the abs function. And Ruffle does seem to separate Value::Number and Value::Integer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However, the test verifies (when 1 is passed or when Number(-0.0) is passed), if it returns int. In other cases, it returns Number. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this should need a special handling per-method. Just like @Lord-McSweeney said, there should probably be a "if the float is integral*, generate a Value::Integer" in (*and within IMO adding missing methods to Number and adding new logic to core Value should go into separate PRs? |
||
Value::Number(-0.0) => Ok(Value::Integer(0)), | ||
_ => Ok(f64::abs(input.as_f64()).into()), | ||
} | ||
} | ||
|
||
pub fn ceil<'gc>( | ||
_activation: &mut Activation<'_, 'gc>, | ||
_this: Value<'gc>, | ||
args: &[Value<'gc>], | ||
) -> Result<Value<'gc>, Error<'gc>> { | ||
let input = args[0].as_f64(); | ||
|
||
if input.is_nan() { | ||
Ok(Value::Number(f64::NAN)) | ||
} else if input.is_infinite() { | ||
Ok(Value::Number(input)) | ||
} else { | ||
let ceiled = input.ceil(); | ||
// Special case: if input was negative and ceil result is 0, preserve -0.0 | ||
if ceiled == 0.0 && input.is_sign_negative() { | ||
Ok(Value::Number(-0.0)) | ||
} else if ceiled >= i32::MIN as f64 && ceiled <= i32::MAX as f64 { | ||
Ok(Value::Integer(ceiled as i32)) | ||
} else { | ||
Ok(Value::Number(ceiled)) | ||
} | ||
} | ||
} | ||
|
||
pub fn floor<'gc>( | ||
_activation: &mut Activation<'_, 'gc>, | ||
_this: Value<'gc>, | ||
args: &[Value<'gc>], | ||
) -> Result<Value<'gc>, Error<'gc>> { | ||
let input = args[0]; | ||
let num = input.as_f64(); | ||
|
||
if num.is_nan() { | ||
Ok(Value::Number(f64::NAN)) | ||
} else if num.is_infinite() { | ||
Ok(Value::Number(num)) | ||
} else if num == 0.0 && num.is_sign_negative() { | ||
Ok(Value::Number(-0.0)) | ||
} else { | ||
let floored = num.floor(); | ||
if floored >= i32::MIN as f64 && floored <= i32::MAX as f64 { | ||
Ok(Value::Integer(floored as i32)) | ||
} else { | ||
Ok(Value::Number(floored)) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
num_ticks = 1 | ||
known_failure = true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1 @@ | ||
num_ticks = 1 | ||
known_failure = true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
num_ticks = 1 | ||
known_failure = true | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably also return
Ok(ret.into())
directly- as Adrian said, it'd be better to keep the Number -> int optimization a general optimization and implement it in a separate PR