Skip to content
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

core: Remove all static strings, shrink Value structs #19716

Merged
merged 11 commits into from
Mar 9, 2025
2 changes: 1 addition & 1 deletion core/src/avm1/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ impl<'a, 'gc> Activation<'a, 'gc> {
// SWF19: "If A is zero, the result NaN, Infinity, or -Infinity is pushed to the stack in SWF 5 and later.
// In SWF 4, the result is the string #ERROR#."
let result: Value<'gc> = if a == 0.0 && self.swf_version() < 5 {
"#ERROR#".into()
AvmString::new_utf8(self.gc(), "#ERROR#").into()
} else {
(b / a).into()
};
Expand Down
5 changes: 3 additions & 2 deletions core/src/avm1/globals/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::avm1::error::Error;
use crate::avm1::property_decl::{define_properties_on, Declaration};
use crate::avm1::{Object, ScriptObject, TObject, Value};
use crate::string::StringContext;
use ruffle_macros::istr;

const PROTO_DECLS: &[Declaration] = declare_properties! {
"message" => string("Error");
Expand All @@ -20,7 +21,7 @@ pub fn constructor<'gc>(
let message: Value<'gc> = args.get(0).cloned().unwrap_or(Value::Undefined);

if message != Value::Undefined {
this.set("message", message, activation)?;
this.set(istr!("message"), message, activation)?;
}

Ok(this.into())
Expand All @@ -41,6 +42,6 @@ fn to_string<'gc>(
this: Object<'gc>,
_args: &[Value<'gc>],
) -> Result<Value<'gc>, Error<'gc>> {
let message = this.get("message", activation)?;
let message = this.get(istr!("message"), activation)?;
Ok(message.coerce_to_string(activation)?.into())
}
21 changes: 12 additions & 9 deletions core/src/avm1/globals/file_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,17 @@ pub fn browse<'gc>(

for i in 0..length {
if let Value::Object(element) = array.get_element(activation, i) {
let mac_type =
if let Some(val) = element.get_local_stored("macType", activation, false) {
Some(val.coerce_to_string(activation)?.to_string())
} else {
None
};
let mac_type = if let Some(val) =
element.get_local_stored(istr!("macType"), activation, false)
{
Some(val.coerce_to_string(activation)?.to_string())
} else {
None
};

let description = element.get_local_stored("description", activation, false);
let extension = element.get_local_stored("extension", activation, false);
let description =
element.get_local_stored(istr!("description"), activation, false);
let extension = element.get_local_stored(istr!("extension"), activation, false);

if let (Some(description), Some(extension)) = (description, extension) {
let description = description.coerce_to_string(activation)?.to_string();
Expand All @@ -262,7 +264,8 @@ pub fn browse<'gc>(
return Ok(false.into());
}
} else {
return Err(Error::ThrownValue("Unexpected filter value".into()));
// Method will abort if any non-Object elements are in the list
return Ok(false.into());
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/src/avm1/globals/netconnection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ impl<'gc> NetConnection<'gc> {
let event = constructor
.construct(&mut activation, &[])?
.coerce_to_object(&mut activation);
let code = AvmString::new_utf8(activation.gc(), code);
event.set(istr!("code"), code.into(), &mut activation)?;
event.set(istr!("level"), istr!("status").into(), &mut activation)?;
this.call_method(
Expand Down Expand Up @@ -189,7 +190,7 @@ fn protocol<'gc>(
.handle()
.and_then(|handle| activation.context.net_connections.get_protocol(handle))
{
Ok(protocol.into())
Ok(AvmString::new_utf8(activation.gc(), protocol).into())
} else {
Ok(Value::Undefined)
};
Expand Down
67 changes: 34 additions & 33 deletions core/src/avm1/globals/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,39 +59,40 @@ pub fn add_property<'gc>(
this: Object<'gc>,
args: &[Value<'gc>],
) -> Result<Value<'gc>, Error<'gc>> {
let name = args
.get(0)
.and_then(|v| v.coerce_to_string(activation).ok())
.unwrap_or_else(|| "undefined".into());
let getter = args.get(1).unwrap_or(&Value::Undefined);
let setter = args.get(2).unwrap_or(&Value::Undefined);

match getter {
Value::Object(get) if !name.is_empty() => {
if let Value::Object(set) = setter {
this.add_property_with_case(
activation,
name,
get.to_owned(),
Some(set.to_owned()),
Attribute::empty(),
);
} else if let Value::Null = setter {
this.add_property_with_case(
activation,
name,
get.to_owned(),
None,
Attribute::READ_ONLY,
);
} else {
return Ok(false.into());
}
if let Some(name) = args.get(0) {
let name = name.coerce_to_string(activation)?;
let getter = args.get(1).unwrap_or(&Value::Undefined);
let setter = args.get(2).unwrap_or(&Value::Undefined);

Ok(true.into())
match getter {
Value::Object(get) if !name.is_empty() => {
if let Value::Object(set) = setter {
this.add_property_with_case(
activation,
name,
get.to_owned(),
Some(set.to_owned()),
Attribute::empty(),
);
} else if let Value::Null = setter {
this.add_property_with_case(
activation,
name,
get.to_owned(),
None,
Attribute::READ_ONLY,
);
} else {
return Ok(false.into());
}

return Ok(true.into());
}
_ => return Ok(false.into()),
}
_ => Ok(false.into()),
}

Ok(false.into())
}

/// Implements `Object.prototype.hasOwnProperty`
Expand All @@ -110,14 +111,14 @@ pub fn has_own_property<'gc>(

/// Implements `Object.prototype.toString`
fn to_string<'gc>(
_activation: &mut Activation<'_, 'gc>,
activation: &mut Activation<'_, 'gc>,
this: Object<'gc>,
_args: &[Value<'gc>],
) -> Result<Value<'gc>, Error<'gc>> {
if this.as_executable().is_some() {
Ok("[type Function]".into())
Ok(AvmString::new_utf8(activation.gc(), "[type Function]").into())
} else {
Ok("[object Object]".into())
Ok(AvmString::new_utf8(activation.gc(), "[object Object]").into())
}
}

Expand Down
6 changes: 4 additions & 2 deletions core/src/avm1/globals/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,14 @@ fn distance<'gc>(
.coerce_to_object(activation);
let b = args.get(1).unwrap_or(&Value::Undefined);
let delta = a.call_method(
"subtract".into(),
istr!("subtract"),
&[b.to_owned()],
activation,
ExecutionReason::FunctionCall,
)?;
delta.coerce_to_object(activation).get("length", activation)
delta
.coerce_to_object(activation)
.get(istr!("length"), activation)
}

fn polar<'gc>(
Expand Down
11 changes: 7 additions & 4 deletions core/src/avm1/globals/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::avm1::property_decl::{define_properties_on, Declaration};
use crate::avm1::{Object, ScriptObject, Value};
use crate::display_object::StageDisplayState;
use crate::string::{AvmString, StringContext, WStr, WString};
use ruffle_macros::istr;

const OBJECT_DECLS: &[Declaration] = declare_properties! {
"align" => property(align, set_align);
Expand Down Expand Up @@ -121,11 +122,13 @@ fn display_state<'gc>(
_this: Object<'gc>,
_args: &[Value<'gc>],
) -> Result<Value<'gc>, Error<'gc>> {
if activation.context.stage.is_fullscreen() {
Ok("fullScreen".into())
let state = if activation.context.stage.is_fullscreen() {
istr!("fullScreen")
} else {
Ok("normal".into())
}
istr!("normal")
};

Ok(state.into())
}

fn set_display_state<'gc>(
Expand Down
6 changes: 3 additions & 3 deletions core/src/avm1/globals/system_ime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::avm1::globals::as_broadcaster::BroadcasterFunctions;
use crate::avm1::object::Object;
use crate::avm1::property_decl::{define_properties_on, Declaration};
use crate::avm1::{ScriptObject, Value};
use crate::string::StringContext;
use crate::string::{AvmString, StringContext};

const OBJECT_DECLS: &[Declaration] = declare_properties! {
"ALPHANUMERIC_FULL" => string("ALPHANUMERIC_FULL"; DONT_ENUM | DONT_DELETE | READ_ONLY);
Expand Down Expand Up @@ -40,11 +40,11 @@ fn do_conversion<'gc>(
}

fn get_conversion_mode<'gc>(
_activation: &mut Activation<'_, 'gc>,
activation: &mut Activation<'_, 'gc>,
_this: Object<'gc>,
_args: &[Value<'gc>],
) -> Result<Value<'gc>, Error<'gc>> {
Ok("KOREAN".into())
Ok(AvmString::new_utf8(activation.gc(), "KOREAN").into())
}

fn get_enabled<'gc>(
Expand Down
16 changes: 13 additions & 3 deletions core/src/avm1/globals/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use crate::avm1::object_reference::MovieClipReference;
use crate::avm1::property_decl::{define_properties_on, Declaration};
use crate::avm1::{Activation, Error, Object, ScriptObject, TObject, Value};
use crate::display_object::{DisplayObject, TDisplayObject};
use crate::string::StringContext;
use crate::string::{AvmString, StringContext};
use gc_arena::Collect;
use ruffle_macros::istr;
use swf::{Rectangle, Twips};

#[derive(Copy, Clone, Debug, Collect)]
Expand Down Expand Up @@ -85,12 +86,21 @@ fn method<'gc>(
Ok(match index {
GET_MATRIX => matrix_to_value(clip.base().matrix(), activation)?,
SET_MATRIX => {
let matrix_props: &[AvmString<'_>] = &[
istr!("a"),
istr!("b"),
istr!("c"),
istr!("d"),
istr!("tx"),
istr!("ty"),
];

if let [value] = args {
let object = value.coerce_to_object(activation);
// Assignment only occurs for an object with Matrix properties (a, b, c, d, tx, ty).
let is_matrix = ["a", "b", "c", "d", "tx", "ty"]
let is_matrix = matrix_props
.iter()
.all(|p| object.has_own_property(activation, (*p).into()));
.all(|p| object.has_own_property(activation, *p));
if is_matrix {
let matrix = object_to_matrix(object, activation)?;
clip.set_matrix(activation.gc(), matrix);
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm1/object/stage_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ fn set_sound_buf_time<'gc>(

fn quality<'gc>(activation: &mut Activation<'_, 'gc>, _this: DisplayObject<'gc>) -> Value<'gc> {
let quality = activation.context.stage.quality().into_avm_str();
quality.into()
AvmString::new_utf8(activation.gc(), quality).into()
}

fn set_quality<'gc>(
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm1/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ impl<'gc> Avm1<'gc> {
// Fire "onLoadInit" events and remove completed movie loaders.
context
.load_manager
.movie_clip_on_load(context.action_queue);
.movie_clip_on_load(context.action_queue, &context.strings);

*context.frame_phase = FramePhase::Idle;
}
Expand Down
15 changes: 2 additions & 13 deletions core/src/avm1/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,8 @@ pub enum Value<'gc> {
}

// This type is used very frequently, so make sure it doesn't unexpectedly grow.
// On 32-bit x86 Android, it's 12 bytes. On most other 32-bit platforms it's 16.
#[cfg(target_pointer_width = "32")]
const _: () = assert!(size_of::<Value<'_>>() <= 16);

#[cfg(target_pointer_width = "64")]
const _: () = assert!(size_of::<Value<'_>>() == 24);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep them separate, to be more explicit? Will be also useful if we ever do NaN boxing on 32-bit.


impl<'gc> From<AvmString<'gc>> for Value<'gc> {
fn from(string: AvmString<'gc>) -> Self {
Value::String(string)
Expand All @@ -48,12 +43,6 @@ impl<'gc> From<AvmAtom<'gc>> for Value<'gc> {
}
}

impl From<&'static str> for Value<'_> {
fn from(string: &'static str) -> Self {
Value::String(string.into())
}
}

impl From<bool> for Value<'_> {
fn from(value: bool) -> Self {
Value::Bool(value)
Expand Down Expand Up @@ -434,9 +423,9 @@ impl<'gc> Value<'gc> {
Value::String(s) => s,
_ => {
if object.as_executable().is_some() {
"[type Function]".into()
AvmString::new_utf8(activation.gc(), "[type Function]")
} else {
"[type Object]".into()
AvmString::new_utf8(activation.gc(), "[type Object]")
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion core/src/avm2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,11 +708,17 @@ impl<'gc> Avm2<'gc> {

// The second script (script #1) is Toplevel.as, and includes important
// builtin classes such as Namespace, QName, and XML.
tunit
let toplevel_script = tunit
.load_script(1, &mut activation)
.expect("Script should load");
init_builtin_system_classes(&mut activation);

activation.avm2().toplevel_global_object = Some(
toplevel_script
.globals(activation.context)
.expect("Script should load"),
);

// The first script (script #0) is globals.as, and includes other builtin
// classes that are less critical for the AVM to load.
tunit
Expand Down
Loading