Reduces "as3XXXX is undefined/null" errors

[yurydelendik]

Reduces amount of "as3XXXX is undefined/null":

• 'TypeError: this.as3Object is undefined' when custom constructor checks if object is called from the attachMovie
• 'TypeError: this.as3Object is undefined' or 'this.as3ObjectOrTemplate is undefined' via refactoring to take custom object deep hierarchy in account
• 'TypeError: as3Offsets is null' by fixing perlinNoise
• 'TypeError: as3Stage is undefined' properly setting stage for AVM1 movies loaded from AS3

[yurydelendik]

/botio test

[shumway-bot]

From: Bot.io (Main)

---

Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://areweflashyet.com:8081/0ddf3b327f7c554/output.txt

[shumway-bot]

From: Bot.io (Main)

---

Success

Full output at http://areweflashyet.com:8081/0ddf3b327f7c554/output.txt

Total script time: 11.93 mins

• Lint: Passed
• Reference tests: Passed
• Trace tests: Passed
• AVM2 tests: Passed
• AVM1 trace tests: Passed
• AVM2 ATS tests: Passed
• Perf tests: Passed

[tschneidereit]

I added quite a bit of comments below. In general I think it would greatly simplify the implementation if the AVM1 builtins could always assume that they have a native (i.e., AVM2) object as their backing store. This could be done by implementing a nativeObject getter that either returns the AVM2 object or a null object that just behaves like one, but essentially just swallows all input.

---

Reviewed 12 of 12 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 9 unresolved discussions, all commit checks successful.

---

^{src/avm1/interpreter.ts, line 902 [r1] (raw file):}
I guess this is the one place where hasAS3ObjectReference as a method would be more complicated. But that can be solved with a check for the method's existence.

---

^{src/avm1/lib/AVM1Globals.ts, line 313 [r1] (raw file):}
What happens here and for all other uses of getAS3Object if this isn't a real display list instance? It throws, right? Would it make sense to return a dummy instance that implements the same interface as the real as3Object but with all methods just doing nothing? Also, and independently, should we have an AVM1Utils.resolveAS3Target<T extends flash.display.DisplayObject>(context): T? There are lots of callsites that do exactly that, but in two calls instead of one.

---

^{src/avm1/lib/AVM1MovieClip.ts, line 371 [r1] (raw file):}
I just tested: this should just return undefined. (Preexisting, but might just as well fix it while you're here.)
It also returns undefined when called on a non-display list object. E.g. MovieClip.prototype.getBounds.call({}) == undefined.

---

^{src/avm1/lib/AVM1MovieClip.ts, line 420 [r1] (raw file):}
Same here.

---

^{src/avm1/lib/AVM1MovieClip.ts, line 573 [r1] (raw file):}
Won't these all throw for non-display list objects?

---

^{src/avm1/lib/AVM1Utils.ts, line 54 [r1] (raw file):}
Why did you turn this from a method to a function? It makes more sense to me to have it be a method. Also, no way of calling it with something that's not of the right type that way.

---

^{src/avm1/lib/AVM1Utils.ts, line 58 [r1] (raw file):}
Can you add a doc comment explaining what this is for? It's far from self-explanatory, and quite important.

With "template" you essentially mean a dummy object, right? If so, the name isn't ideal, because it's not something that can be used to create an actual instance. "Dummy" or "Placeholder" would make more sense.

Also, I don't think this needs to be part of the function name: as far as the AVM1 implementation of the builtin methods is concerned, it's an implementation detail. Perhaps getNative? (That'd work much better if this function were a method, which I would prefer for other reasons anyway. See the comment for getAS3Object.)

---

^{src/avm1/lib/AVM1Utils.ts, line 62 [r1] (raw file):}
s/a ASObject/an ASObject/
Also, it doesn't really explain what's going on here. I guess a good comment for the whole function should solve that, though.

---

^{src/avm1/lib/AVM1Utils.ts, line 277 [r1] (raw file):}
Do we really need the isNullOrUndefined here? Why not just this._as3Object ? this._as3Object.name : undefined? _as3Object is guaranteed to be an object if it exists at all.

---

Comments from the review on Reviewable.io

[yurydelendik]

Review status: all files reviewed at latest revision, 8 unresolved discussions, all commit checks successful.

---

^{src/avm1/interpreter.ts, line 902 [r1] (raw file):}
avm1IsTarget shall be refactored to not rely on Lib.hasAS3ObjectReference. I'm keeping using isAVM1Instance just to continue pass tests.

---

^{src/avm1/lib/AVM1Globals.ts, line 313 [r1] (raw file):}
It would throw, yes. We must be careful about use dummy as3Object, I'll investigate that later. resolveAS3Target must be refactored.

---

^{src/avm1/lib/AVM1MovieClip.ts, line 371 [r1] (raw file):}
out of scope of this PR

---

^{src/avm1/lib/AVM1MovieClip.ts, line 573 [r1] (raw file):}
Yes, I'll investigate that later.

---

^{src/avm1/lib/AVM1Utils.ts, line 54 [r1] (raw file):}
We cannot use any methods/properties of the object until it's constructed using attachMovie -- that was the reason of the as3Object/as3ObjectOrTemplate error messages. The methods or properties must be attached to the object in the prototypical chain only when constructed by attachMovie and state shall be saved there (instead of intermediate link)

---

Comments from the review on Reviewable.io

[tschneidereit]

Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

^{src/avm1/lib/AVM1MovieClip.ts, line 371 [r1] (raw file):}
Yes, but will be forgotten - or you can just delete the line and write return undefined; ;) (And no, I'm not generally advocating for making unrelated changes. This one isn't really unrelated, as it would also fix a reason for throwing under superficially similar instances. Also, it's very, very simple.)

---

^{src/avm1/lib/AVM1Utils.ts, line 54 [r1] (raw file):}
I'm not sure I agree. That depends on what exactly you mean by "we cannot use [..]". They certainly exist and are callable, and don't throw. The methods exist on instances created with, e.g., new MovieClip() and can be called. They just don't do anything.

For all uses of getAS3Object, getAS3ObjectOrTemplate, and _as3Object, we should return a lazily created dummy object that has all the right methods, none of the properties (because they're all just undefined until you set them to something in AS1/2), and where the methods do nothing. Then there's no need to ever do any checking.

---

Comments from the review on Reviewable.io

[yurydelendik]

/botio test

---

Review status: 11 of 17 files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from the review on Reviewable.io

[shumway-bot]

From: Bot.io (Main)

---

Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://areweflashyet.com:8081/c161e376bf85ea5/output.txt

[shumway-bot]

From: Bot.io (Main)

---

Success

Full output at http://areweflashyet.com:8081/c161e376bf85ea5/output.txt

Total script time: 11.94 mins

• Lint: Passed
• Reference tests: Passed
• Trace tests: Passed
• AVM2 tests: Passed
• AVM1 trace tests: Passed
• AVM2 ATS tests: Passed
• Perf tests: Passed

[tschneidereit]

Nice, thank you. r=me with the feedback on comments addressed.

---

Reviewed 6 of 6 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

^{src/avm1/lib/AVM1Utils.ts, line 51 [r3] (raw file):}
Can you add something like "Returns false for MovieClip instances or instances of constructors with MovieClip on their prototype chain that were created in script using, e.g., new MovieClip(). Those lack the part of their internal structure that makes them displayable."
Without that, the comment only states what the function name already explains.

---

^{src/avm1/lib/AVM1Utils.ts, line 58 [r3] (raw file):}
Either change "object" to "obj", or rename the parameter.

---

^{src/avm1/lib/AVM1Utils.ts, line 66 [r3] (raw file):}
Same here.

---

^{src/avm1/lib/AVM1Utils.ts, line 68 [r3] (raw file):}
"If the reference doesn't exist, obj was created in script, e.g. with new MovieClip(), and doesn't reflect a real, displayable display object. In that case, an empty null-proto object is created and returned. This is used for classes that are linked to embedded symbols that extend MovieClip. Their inheritance chain is built by assigning new MovieClip to their prototype. When a proper, displayable, instance of such a class is created via attachMovie, initial values for properties such as tabEnabled can be initialized from values set on the template object."

---

Comments from the review on Reviewable.io