core: Use Vec::retain in Avm1TextFieldBinding::bind_variables

[SuchAFuriousDeath]

Description

Just a tiny refactor, std::mem::take has to be used to satisfy the borrow checker.

SIDENOTE: The TODO talks about Vec::drain_filter, which got stabilized as Vec::extract_if, but retain is the better fit here, since there is no resulting iterator we have to consume.

Checklist

• I, a human, have self-reviewed this PR and fully understand the changes within.
• I have made or updated tests where possible.
• All of my commits are properly scoped, compile successfully, and pass all tests.
• This PR does not make sense to split up into smaller PRs.
• An LLM was involved in the authoring of this code.

[kjarosh]

Just a general note about this particular code (text variable binding): it's generally untested and buggy in Ruffle, it can easily introduce regressions.

[SuchAFuriousDeath]

@kjarosh 🥺

[kjarosh]

This will change the order of elements, does it matter?

[SuchAFuriousDeath]

Good question, I didn't consider that. From looking at the code, I don't see a reason why it should matter.

[kjarosh]

For instance, try_bind_text_field_variable calls object.set which calls watchers. Can we add at least a test that checks the order of called watchers?

[SuchAFuriousDeath]

I had Claude vibe out a test, see if it makes sense please, it does to me, but this isn't really an area of code I'm familiar with.

[kjarosh]

Is the test failing with the previous code?

[SuchAFuriousDeath]

No, I just checked. It doesn't depend on the order of iteration I guess.

[SuchAFuriousDeath]

However, I think your claim here:

For instance, try_bind_text_field_variable calls object.set which calls watchers.

Is wrong because try_bind_text_field_variable is called with false as the second argument and that results in watchers not being called, at least based on my understanding.

[SuchAFuriousDeath]

@kjarosh 🥺