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

refactor MovieClip to fix several bugs #1607

Closed
wants to merge 1 commit into from
Closed

refactor MovieClip to fix several bugs #1607

wants to merge 1 commit into from

Conversation

sbimikesmullin
Copy link
Contributor

@sbimikesmullin sbimikesmullin commented Jun 10, 2017

Refactor MovieClip.__enterFrame() and friends to fix:

  • FrameObject CREATE duplication
  • FrameObject REMOVE failures,
  • bugs related to transposed zero-indexed/one-indexed frame number references
  • bugs in frame script playback synchronization with animation

Added a ton of comments to improve maintainability.

Tried to code in a way that made it more obvious what was happening, with only mild performance optimizations in mind. Should be as-fast or faster than before.

The only major change is that all MovieClip.__children:DisplayObject instances are initialized from MovieClip.new() constructor now. This happens once instead of multiple times throughout animation loop/playback. We believe its a sensible approach inspired by other implementations, and a performance optimization (read comments in patch for detail).

The primary reason we needed this was to eliminate several bugs that were breaking playback for a very complex set of .SWF files we had imported using SWFLiteExporter and were playing back at runtime in the browser with OpenFL on the HTML5 target.

There are still issues to be resolved with the Filters and Blend Modes, but we'll get to those soon.

💪 credit @sbimikesmullin with minor contributions from Elvir Tatarevic and @sbijoshj.

💡 Interested in whatever feedback good/bad you have on this one @jgranick. Could go over it in detail on call if you want, and show before/after behavior.

🛑 ✋ 👮 WARNING: This PR depends on #1599 and #1601, as written. You should merge those first. They may cause this one to conflict but shouldn't because they have the exact same lines.

@@ -47,7 +47,7 @@ class MovieClip extends flash.display.MovieClip {
@:noCompletion private var activeObjects:Array<ChildObject>;

#if flash
@:noCompletion private var __currentFrame:Int;
@:noCompletion private var __currentFrameOneIndexed:Int;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👷 🚧 WARNING: I do not feel I understand the tools version of the MovieClip class, and while these changes seem to satisfy the build, you should really check them to be sure the original intent is still functional. With or without them, I couldn't tell the difference.

It would help to get an explanation from you @jgranick here.

if (null != duplicate) {

trace("Multiple CREATE tags for the same characterId and depth are being merged.\n"
+ "Asset \""+ untyped this.__swf.library.rootPath +"\", "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this trace statement should be removed once done debugging


if (null == symbol) {

trace("Unable to CREATE DisplayObject instance; got NULL symbol\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

once we figure out why null symbols exist (you may already know, i haven't had time to figure out yet), we should remove this unless/except debugging.

// TODO: find out why some displayObject instances are null
if (null == displayObject) {

trace("Unable to CREATE DisplayObject instance; got NULL __createObject()\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here

displayObjectCache = frameObjectEntry.displayObjectCache;
}
if (null == displayObjectCache || null == displayObjectCache.displayObject) {
trace("Tried to UPDATE a DisplayObject child that hasn't been CREATED yet. "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this may benefit from an #if debug block

@sbimikesmullin
Copy link
Contributor Author

rebasing this one now

several bugs related to zero/one-indexed frames, and synchronizing frame
playback
@jgranick jgranick closed this in b1baeaa Jul 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant