avm1: fix buttons that were not clickable after a goto

[Flawake]

It is my first commit in this repo were I changed a serious amount of code.
This fixes issue #1108 and all underlying issues:
#8482, #8293, #9966, #10789, #7903, #7170, #6261, #4402
This is being done by placing removed displayObjects into a BTreeMap so we can use them again if there is a goto.
When the next frame begins the BTreeMap is emptied.

[adrian17]

Pinging @kmeisthax / @Herschel , to me this feels like a hack that happens to work but isn't really how it's supposed to work under the hood - and doesn't feel like it matches Mike's original explanation from #1108 either? I can't say with certainty, I just have a bad hunch about it.

Also, we definitely want some extra tests for this.

[kmeisthax]

So, the approach being taken here is to grab buttons out of the display tree but restore them if we need to recreate them. Mike explains this in terms of events being fired or not fired, but it doesn't have to be literally that. On the other hand, holding old buttons so we can bring them back does seem hacky.

Toad06's tests from issue #1108 would probably solve this dilemma. I don't see them incorporated into this PR - it might be helpful to see if they pass with these changes or not.

[iwannabethedev]

That sounds good, if you have made extensive changes, I can try to take a look at it, but I think adrian17 and kmeisthax should also look at it at some point if you have made extensive changes.

[kmeisthax]

It appears the new setup for this now is to check if the same object got reinstantiated; this seems viable. I'd approve and merge, but I did notice one flaw with how this is implemented: CharacterId isn't globally unique; you need to check both what character ID a display object was instantiated from and what movie that character came from. If you load two movies into the same stage, they both can have a "button 5", and this will falsely skip rollovers for them.

(If Flash actually doesn't check the source movie somehow, and multi-movie loads allow buttons from different movies to substitute for one another, ignore the following comments and instead add a NOTE comment documenting this.)

[kmeisthax]

One more thing: you need to compare SwfMovies by pointer instead of URL, since you can have two different movies with the same source URL.

[Toad06]

Toad06's tests from issue #1108 would probably solve this dilemma. I don't see them incorporated into this PR - it might be helpful to see if they pass with these changes or not.

@kmeisthax These tests are edited with JPEXS and based on the file available in #4402 (as I don't have a Flash editor to make them properly). I only made them for quick manual testing purposes, so I think we shouldn't add them as part of the Ruffle test suite (each file is about 2mb and includes data that will not be relevant here - without mentioning potential copyright issues).

[kmeisthax]

LGTM! I'm going to merge.

@Toad06 Gotcha on the tests, the new version of this PR has enough as is that I don't think we'll need yours anyway.

[kmeisthax]

So... it appeared the rebase failed and @adrian17 tried to clean it up. In Discord, he had a question I couldn't exactly answer: why do we need to run find_first_character_instance on down_object?

I'm assuming there's some subtlety of how buttons survive gotos that requires searching the whole display tree for a matching copy of the object, but I can't exactly prove that. Furthermore, the new_over_object code doesn't actually do this check, it just checks the id, depth, and movie for equality and then copies the state if it matches. Is there any particular reason why mouse_down_object can't work the same way?

Also, I noticed a bug in find_first_character_instance while trying to answer Adrian's question.

[Flawake]

@kmeisthax we are seraching for the object because of this: when we are holding the button down and moving out of the button area. We should keep the button in it's mouse down state, so we need to check if the object is still there. We also can't check if the over object points at the button since the over object should remove it's reference when not hovered anymore.

[n0samu]

I'm not experienced with reviewing PRs, but I read through the diff and tested it out, and everything is working well, plus the code looks sensible and fine. I think it's time to merge this! And thank you for your work!