Rethink the concept of plugins / get rid of them

[Gama11]

Plugins are not a very useful concept. They're not different from FlxBasics, yet they behave differently - using a kind of singleton pattern where only one instance of the plugin can exist at a time. This creates some interesting issues with substates, as plugins have no concept of belonging to a state like everything else in flixel does. This means tweens in a parent state won't stop if a substate is opened, even if everything else pauses.

It would be much more flexible, intuitive and powerful if plugins were add()ed to states like everything else.

[sruloart]

If a plugin is added like everything else per state, how is it still a plugin? off with its head I say...

[Dovyski]

The way I see things, plugins should use Flixel API to perform actions. The plugin itself is instantiated and lives in the "plugin list" (wherever it is).

From there, the plugin instance can perform its actions. A very rudimentary example: if a plugin needs to draw lines and squares on the screen (a debug plugin), it will most likely add a FlxSprite to the current state and use that sprite to make all the drawings.

[Gama11]

@Dovyski And why is it beneficial to have that plugin in a separate list instead of state.members, where every other flixel object is added to? And reduce the amount of possible instances to 1?

[Dovyski]

If you keep plugins in a different place, they are not related to the state itself. Maybe a plugin wants to handle input and do something with that (it doesn't have to be on the screen in order to achieve that).

If you keep plugins in the state, what should happen when a new state comes? Are the plugin instances removed? Are they transferred to the new state?

[Gama11]

The fact that plugins are not related to the state is what's creating this issue in the first place. The timer, path and tweener manager don't really work too well with substates / pausing. They have some logic to be cleared on state switches anyway.

[Gama11]

Quick breakdown:

Problem

The way plugins are handled makes them incompatible with substates. You use substates to pause the parent state, however, tweens, paths or timers that are created in that state are not paused because they are handled globally. The same applies to plugins like FlxMouseEventManager, it can call callback functions of sprites that are currently in a paused state.

Solution

Plugins need to be attached to their state in some way. There are several ways this could be achieved, here's my proposed solution:

Turn FlxPath, FlxTimer and FlxTween into FlxBasics

If these objects extend FlxBasic, there's no need for a manager class. They can add() themselves to the currently active state, and also remove() themselves when they are finished.

For plugins like FlxMouseEventManager, the user would need to create an instance, keep a reference to it and add() it like any other objects. We would get rid of all static functions in that class and turn them into member functions. I think that this approach is more intuitive and more flexible than the current singleton-like-one.

Pros:

• Reduces code complexity - all objects are FlxBasics, there's no more special case for these particular ones. Manager classes are no longer needed.
• Effectively gets rid of the plugin system which doesn't seem particularly useful and very rigid.

Cons:

• These objects would inherit several things from FlxBasic that are of no use for them, like visible, collisionType or draw() (although the latter would be useful for FlxPath debug drawing). However, this is arguably already the case for current plugins, FlxObject etc...
• This would be the first time flixel manipulates state.members itself. The user could have made assumptions that would cause conflicts with this (e.g.: I just added a sprite, therefore state.members.length == 1 must return true).

[KeyMaster-]

I'm all for the approach of linking plugins with the state they belong to, but making them objects seems a bit strange.
For me objects are only "things" in the game, plugins are more like services. I think there might be a better solution, but making them FlxBasics might be good because it keeps in line with coherent object structure.

[kevinresol]

+1 for attaching plugins to a state
Can it be a separate array for plugins while members still holds ordinary objects like FlxSprite?

[Gama11]

@KeyMaster- Plugins already are FlxBasics, that's part of why the plugin system is confusing - requiring them to be used in a certain way seems arbirtary. You could already add() plugins to your state (the same goes for FlxSubState and FlxState btw, since those already extend FlxBasic, but that's a different issue). What's not a FlxBasic currently are the objects the plugins manage: FlxPath, FlxTimer and FlxTween.

If you think about it, FlxGroups are also not "things in your game", they are merely containers for them, yet you can add them.

@kevinresol I thought about that as well. But why is it beneficial to have "plugins" be a separate concept from everything else? It seems like they would work nicely with the already exisiting logic for state members.

[Gama11]

Btw, we have a similar issue with FlxSound - it also extends FlxBasic, yet is managed in a different list (FlxG.sounds.list).

[kevinresol]

I agree that plugins should not be a separate concept. i think the problem is about the implementation of FlxBasic and FlxObject. We should clear the understanding what FlxBasic really is.

[Gama11]

@kevinresol You're right, the concept of FlxBasics themselves is somewhat flawed. In the docs it says:

Has no size, position or graphical data.

Yet it has a visible flag and a draw() function, for convenience I guess?

[kevinresol]

I think FlxBasic should be a class that only provide the update() function, and basic stuff like:

• active to tell if it should be updated or not.
• exists for recycling

[kevinresol]

Because FlxBasic is really an abstract concept for programming purpose. Only at FlxObject level it should have properties like alive, visible, draw() as it is a representation of concrete objects

[Gama11]

@kevinresol I guess ideally, FlxBasic would be FlxUpdatable. But how do you manage drawable objects then? Create a FlxDrawable class that extends it? FlxObject would extend FlxDrawable and FlxGroup would accept FlxDrawable in its add()?

Technically, FlxObject has no graphical data either, just FlxSprite and FlxTilemap that extend it. It only needs a draw() to draw the bounding box.

[JoeCreates]

Would be so much nicer in a more component based architecture. :D

[Dovyski]

The fact that plugins are not related to the state is what's creating this issue in the first place. The timer, path and tweener manager don't really work too well with substates / pausing. They have some logic to be cleared on state switches anyway.

Can't those plugins monitor signals and react to them (e.g. state pushed)?

[Gama11]

@Dovyski I don't see how that solves the underlying problem?

[Dovyski]

Using the FlxMouseEventManager as an example. If it is monitoring a state push signal, it knows which state is active, so it's possible to selectively invoke callbacks of the entities in the active state only.

If you attach plugins to the state, a developer will never be able to create a plugin that works globally on Flixel (e.g. apply visual effects to every frame). For every new state created, a developer must stop and think which plugins should be instantiated and added to that state.

[Gama11]

@Dovyski If you do it that way, you then you have to store the state a sprite belongs along with it in the mouse event manager. That's exactly the same as adding them to the state, you're just storing the information in a different way (a way that seems backwards to me / not preferable). For that you'd first have to figure out which state the sprite belongs to, and that's pretty tricky, as sprites in flixel don't have a concept of a parent and can be added to multiple groups (even multiple states).

I don't think there should be a plugin that applies effects globally. That should be a per-camera thing.

[Dovyski]

You have a point, @Gama11 You could fix that in FlxMouseEventManager, for instance, by adding entries and, internally, the manager adds a reference to the current state.

That fix, however, is a re-implementation of states and entities as you mentioned.

[gamedevsam]

I can see potential order of operation issues if we add FlxTimer, FlxTween, FlxPath, and FlxMouseEventManager to the state (since they currently get updated / drawn before the state), it might not cause any issues, but we have to run some tests to make sure we don't break anything.

FlxSound is not a plugin, so it shouldn't be part of this discussion.

[impaler]

Putting plugins in the state as suggested is a huge change to the most common content in members. This is like asking to use non display objects in the flash api like flash.sprite.addChild(myPluginToUpdate), this is very confusing and inconsistent.

"incompatible with substates" can be solved instead by moving a plugins collection into each FlxState and substate itself. There you could easily state.pause() iterate through all that state's plugins to pause/unbind their signals, then state.resume().

As Jens points out state.members.length wouldn't be very useful anymore. The user would need to keep a members array for FlxSprites. It would be much nicer if states just have a new collection for plugins that you would use with state.addPlugin.

Plugins could simply be IFlxPlugin with pause, resume, destroy to handle the unbinding/binding the methods from the signals automatically. Keeping plugins in their own collection also lets any issues @gamedevsam could be overcome as you can update them when you need them as they are actually separated from the flixel sprites. However Sam raises a good point in that I havent looked too much but I dont think FlxSignals offer any priority api which could be added.

[Gama11]

@impaler That's a valid concern. However, if you think about it, FlxGroup is also not a display object, it's merely a container, yet we add() it to the state. So in that regard, flixel's system is already flawed / "unpure".

I don't like the idea of creating a separate plugin collection for states very much, it feels like a bit of a hack to me. Timers, tweens and paths are not really "plugins", they are an integral part of the engine.

Another, signal-based solution is imaginable. FlxState would need an update signal that timers, paths and tweens can subscribe to. This would be extremely simple to implement and only require minimal changes to the code. The only concern I have with that approach is performance, as callback functions tend to be slower than "actual" function calls IIRC. (@gamedevsam?)

[Gama11]

There's another issue that has not been considered yet and applies to all solutions: The current workflow with timers, paths and tweens is pretty simple, since they manage everything themselves. However, if we allow multiple states to be active at the same time (see #1091 and #1084), this won't be possible anymore, since we have no way of knowing which state an object should be associated with.

[gamedevsam]

We could bring back the concept of "pre-update", and "post-update" as signals contained by states. And only use those for Timers / Tweens / Paths. Triggering signals is slower than doing direct function calls, but I think for objects with limited life spans, it would be OK. We'd need to evaluate performance difference though, so having a Tween / Timer performance test might not be a bad idea.

[impaler]

@Gama11 most flash projects use empty sprites which act as containers like flixel uses FlxGroup, this isnt an uncommon or unpure thing. I think your battling with the deep inheritance while wanting to separate logic to particular parts.

Whether you call them plugins/managers whatever, do you want one members array to update everything imaginable, or do you want to separate the display objects and the managers/plugins? It seems more of a hack like you say to put everything in one place.

I think its better organization of code for them to be handled and organized separately, whether that is with signals which are just collections of callbacks, or more simple containers.

[Gama11]

I should probably mention that @JoeCreates is working on a solution for this (just to two people working on the same thing and duplicating efforts).