core/avm1/avm2: Remove some static AvmStrings #19272
Conversation
Lord-McSweeney
force-pushed
the
avmstring-less-static
branch
2 times, most recently
from
dab3010 to
1cde56d
Compare
Lord-McSweeney
added
A-core
Lord-McSweeney
force-pushed
the
avmstring-less-static
branch
from
d543df4 to
c6c781b
Compare
Lord-McSweeney
added
the
waiting-on-review
| self.base().name().unwrap_or_default() | ||
| } | ||
| fn name_optional(&self) -> Option<AvmString<'gc>> { | ||
| fn name(&self) -> Option<AvmString<'gc>> { |
There was a problem hiding this comment.
fwiw, from the API pov, it does make sense for a DO to always have a name, I think?
We'd need to check, but maybe it's possible that a DO is always suppposed to have some name (see set_default_instance_name)?
With that it mind, maybe eventually this still should return an AvmString and unwrap internally?
That said, I haven't tested any of this. I'm not suggesting to make the refactor bigger, rather want to avoid refactoring a dozen places to support an Option, which might be reverted in the future.
There was a problem hiding this comment.
No, not all DOs have names. Graphics that were placed without a name, for example, don't have names. This was what caused the bug that #14463 fixed: you cannot look up a Graphic child with any name, even though if you somehow got access to a Graphic in AVM1 its .name would be an empty string.
| ) -> EventObject<'gc> { | ||
| let event_type = AvmString::new_utf8(activation.gc(), "netStatus"); |
There was a problem hiding this comment.
We should do interning in most of these places, also for key.
Lord-McSweeney
force-pushed
the
avmstring-less-static
branch
3 times, most recently
from
8150ad3 to
0dd73d5
Compare
Lord-McSweeney
force-pushed
the
avmstring-less-static
branch
from
0dd73d5 to
e30d2da
Compare
Replaces the old `From<&'static WStr>` impl on AvmString
1. We no longer create a static AvmString to compare broadcast event types to to ensure they're a whitelisted event. 2. We now allocate an AvmString for looking up builtin classes before storing them on SystemClasses/SystemClassDefs.
…Into<AvmString>`
Lord-McSweeney
force-pushed
the
avmstring-less-static
branch
from
e30d2da to
8e8e6a0
Compare
Only about 120 static strings left in AVM2!