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

[next] Return of the Stage #4586

Closed
wants to merge 1 commit into from
Closed

[next] Return of the Stage #4586

wants to merge 1 commit into from

Conversation

ivanpopelyshev
Copy link
Collaborator

@ivanpopelyshev ivanpopelyshev commented Jan 6, 2018

Work in progress - need reviews

  • New name for InnerStage class.
  • New name for animationDeltaMax

What if we rename detachChild into removeLater , silendRemoveChild or stealChild ?

The time has come! Russian Christmas update!

Follow #4179 , I kept it very simple.

This is colossal PR that was waited too long, people really want correct added and removed events and a way to track all animations that exist in main stage tree. This is critical for projects that need to free resources and count references to them, also for projects that require managable tweens and animations.

InnerStage tracks sets of DisplayObjects and Containers that exist in the stage, some in detached stage. 170 lines of serious algorithm code were always on used-side.

parentStage property always points to the stage where the object is registered.

As a default implementation that can be easily changed in plugins and in user-side code, any object that has animate method will be called when the time comes. It can be changed both by overriding Stage methods and handling added and removed events.

There can be nested stages.

Better Events

added and removed are fired for whole subtree.

var stage = new PIXI.Stage();
var container = new PIXI.Container();
var child = new PIXI.DisplayObject();

container.addChild(child); // nothing
stage.addChild(container); // child is ADDED!

Additional way of moving objects in the same stage without firing events: detachChild. It exists in cocos2d-x as a flag to removeChild, which is not verbose.

var container1 = new PIXI.Container();
var container2 = new PIXI.Container();
var stage = new PIXI.Stage();

stage.addChild(container1);
stage.addChild(container2);

var child = new PIXI.Container();
container1.addChild(child); //ADDED
container1.detachChild(child);// event doesnt fire
//some calculations
container2.addChild(child); // event doesnt fire because its in the same stage
//some calculations
container2.detachChild(child);

stage.innerStage.flushDetached(); // actual REMOVED, we dont need related resources anymore!

Generated Resources: Dispose vs Destroy

Alternative to destroy method, just override onRemoved or register a handler.

This snippet automatically frees videomemory of unstaged text:

var text = new PIXI.Text('stuff');
text.on('removed', () => { text.texture.baseTexture.dispose(); } );

Fallback

Use PIXI.Container as a root.

app = new PIXI.Application( { stage: new PIXI.Container() } );

added and removed events wont be fired in that case.

Solution for remove at animate problem

Suppose you animate spine object or AnimatedSprite and want to remove that thing from the stage from inside of event.

myAnimatedObject.on('someEventInsideAnimation', () => {
    if (myAnimatedObject.dead) 
    {
         myAnimatedObject.parent.removeChild(myAnimatedObject);
    }
});

That can really affect neighbour elements, or even throw an exception. Any event fired from inside animation or old updateTransform can trigger that response.

With detachChild it will work just fine.

@ivanpopelyshev ivanpopelyshev changed the title Next stage Return of the stage Jan 6, 2018
@ivanpopelyshev ivanpopelyshev changed the title Return of the stage [next] Return of the stage Jan 6, 2018
@ivanpopelyshev ivanpopelyshev changed the title [next] Return of the stage [next] Return of the Stage Jan 6, 2018
@ivanpopelyshev
Copy link
Collaborator Author

ivanpopelyshev commented Jan 22, 2018

@endel original Added/Removed events PR author , #1870

Flash has two type of events: added and added_to_stage, right? Do we need both or only added_to_stage that i want to make default in this PR?

Just a reference, found your code related to stage: https://github.com/gamestdio/pixi-engine/blob/master/src/PixiPatches.ts#L8

@endel
Copy link
Contributor

endel commented Jan 22, 2018

Hi @ivanpopelyshev,

Here's the description of "added_to_stage" event, from Flash:

event.target: The DisplayObject instance being added to the on stage display list, either directly or through the addition of a sub tree in which the DisplayObject instance is contained. If the DisplayObject instance is being directly added, the "added" event occurs before this event.

AFAIK, it's not possible to have Stage as a child of another Stage on Flash. It's always used only as the root element.

On pixi-engine, I've monkey-patched the addChild/removeChild methods to expose the "added"/"removed" events globally and catch them from another scope, because they don't bubble / propagate on pixi.js.

On Flash the events always bubble through the whole display list, and it is possible to listen for events from the root (or any of its parents), and manipulate any deeper DisplayObject added, which comes as target of the event.

IMHO having a detachChild method would cause more confusion. Can we have a real world example of when detachChild would be necessary? 😅

Cheers!

@@ -52,6 +52,8 @@ export default class Application
* for devices with dual graphics card **webgl only**
* @param {boolean} [options.sharedTicker=false] - `true` to use PIXI.Ticker.shared, `false` to create new ticker.
* @param {boolean} [options.sharedLoader=false] - `true` to use PIXI.Loaders.shared, `false` to create new Loader.
* @param {PIXI.Container} {options.stage} - Pass existing stage or container
* @param {number} {options.animationDeltaMax} - How many frames will be processed by animation if user switches the tab
Copy link
Member

Choose a reason for hiding this comment

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

55-56 missing dots :)

Copy link
Member

Choose a reason for hiding this comment

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

ok, I think this can be addressed as separate PR, we can add this to CI or somewhere


onRemoved()
{
// Text frees videomemory when it leaves the stage
Copy link
Member

Choose a reason for hiding this comment

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

let's move to the docblock?

import Runner from 'mini-runner';

/**
* Stage is a Container that takes care of add/remove events and animations
Copy link
Member

Choose a reason for hiding this comment

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

As for me, I don't quite get a reason to have multiple stages.

Would be cool to add short example/use-cases in docs?

Copy link
Member

Choose a reason for hiding this comment

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

I think something like your description is a great thing to put here.

Just maybe make it more compact.

faster detach

more detach improvements

tests are failing. Its ok, those are really good tests.

tests are fine!

ParticleContainer does not spam the stage.

More animation parameters

Text optimization

empty functions lint

multistage

animation detach test

animated removal solved!

onAnimate fixes

derpy mcderp

animate through nested stage
@bigtimebuddy
Copy link
Member

@GoodBoyDigital and I have discussed and we have decided to close this omnibus PR because of the unwanted complexity it would add to v5, it's incompleteness (failing tests), added API surface, and a failure to see community enthusiasm for this approach.

However, there may be smaller incremental PRs within this one (e.g., the ability to provide a "stage" container for Application options, adjustments to inherited animation time/ticks, a way to add/remove children without events) but overall all the functionality within this Stage implementation is probably best as an external plugin or system for Pixi.

@lock
Copy link

lock bot commented May 22, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators May 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants