Starling.current doesn't return always the right value

[pol2095]

Hello,

Starling.current doesn't return always the right value, I understand it is there for convenience, a majority of applications only need one Starling instance but it doesn't work with multiple Starling instances (Air NativeWindow).

I suggest to add a "starling" value in starling.display.DisplayObject

in starling.core.Starling, line(271)

mStage = new Stage(viewPort.width, viewPort.height, this, stage.color);

in starling.display.Stage, line(72)

private var mStarling:Starling;

override public function get starling():Starling { return mStarling; }

/** @private */
public function Stage(width:int, height:int, starling:Starling, color:uint=0)
{
    mStarling = starling;
    ...

in starling.display.DisplayObject, line(1004)

public function get starling():Starling { return stage.starling; }

see this discussion NativeWindow bug with feathers

Thanks

[PrimaryFeather]

Thanks a lot for the suggestion!

I know that Starling.current is not ideal — I despise static globals like those normally, because they often cause problems. But in this case, not having it would make life so much more difficult for Starling developers (most of which only use one instance, anyway), that I had to make an exception.

The way this works currently is that in all steps of the Starling per-frame logic, setCurrent() is called. That way, in each render method, Starling event handlers, juggler callbacks, etc., the current property is set correctly. So if you rely on those, you'll rule out many problems right away; e.g. you can use juggler.delayCall instead of setTimer or new Timer(), etc.

That's just to explain how it currently works, your request is something else, of course. :smile:

So, it might make sense to add such a problem to the "stage" (or, alternatively, to add a convenience method to Starling to get from a stage to a Starling instance). I'll consider that, and maybe discuss this with Josh, as well.

However, on the DisplayObject class, it's not a very good idea, IMHO. That's because the stage property is only accessible once the object is connected to the stage, i.e. it returns null while the object is not part of the display tree. And very often, people set up their objects in the constructor — in which they wouldn't be able to get the Starling instance then.

And even if we'd have that property, we run into problems elsewhere: just think about textures. When you call Texture.from...(), that method will need the Starling context — which it can get via Starling.current.context, of course. So even if the display object would have access to the Starling instance, the Texture class hasn't. The only workaround would be to pass the Starling instance (or the context) to the from...-method, as well. And that would be necessary for a huge number of classes: you'd constantly be carrying around the Starling instance; a very painful way to code. :-/

So, even adding such a property to the Stage class (which I will consider nevertheless), most of the code would still have to rely on Starling.current, so we wouldn't gain much. Thus, I think the current way of doing it is the best compromise we've got. :wink:

[pol2095]

OK, the Feathers solution stageToStarling() work fine, it's not necessary to add my suggest.

[PrimaryFeather]

Thanks for your feedback! I also think that's the best solution - although we might move that from Feathers to Starling.

[joshtynjala]

I think a starling property on the Stage class would feel cleaner to me than the utility function.

[BigPhilCombo]

I agree with Josh. The stage should just have a reference to its starling instance.

In regard to keeping legacy '.current'. You keep a list of of instances, if there are less than 2 instances it behaves as it does today. If there are more than one instances you trace out something in the getter function along the lines of "Depreciated double-check that this is the starling instance you want. User stage.starling instead" but still return whatever value was just set from makeCurrent()

[joshtynjala]

I don't agree that Starling.current should be considered legacy or deprecated. I plan to use Starling.current extensively as a fallback in situations where a display object's stage property is null. Also, as Daniel mentioned, Starling.current is used in many places where there is no access to the stage and it would be cumbersome to require people to pass it in.

[BigPhilCombo]

Fair enough but it might be helpful to indicate to a user that the value of Starling.current might not be the value they are expecting.

On Fri, May 6, 2016 at 3:49 PM, Josh Tynjala notifications@github.com wrote:

I don't agree that Starling.current should be considered legacy or deprecated. I plan to use Starling.current extensively as a fallback in situations where a display object's stage property is null. Also, as Daniel mentioned, Starling.current is used in many places where there is no access to the stage and it would be cumbersome to require people to pass it in.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Gamua/Starling-Framework/issues/857#issuecomment-217541670

Phillip Chertok

[pol2095]

If it's possible, add just this

in starling.core.Starling, line(271)

mStage = new Stage(viewPort.width, viewPort.height, this, stage.color);

in starling.display.Stage, line(72)

private var mStarling:Starling;

public function get starling():Starling { return mStarling; }

/** @private */
public function Stage(width:int, height:int, starling:Starling, color:uint=0)
{
    mStarling = starling;
    ...

[PrimaryFeather]

That should do it! I hope that helps. :smile:

In any case, remember: this is no silver bullet. Even when you use this method and thus make sure that you're working with the right instance, Starling will internally still use Starling.current; there's simply no way around that while keeping the API comfortable to use. If in doubt, however, you can now call stage.starling.makeCurrent() to make sure any subsequent code uses the right instance.