Added a way to instantiate Controllers by using factories

[BorealFeast]

-Added a way to create controllers other than using the default Constructor by registering a Factory to a specified Controller class type. If no factories are found for a controller type, the default constructor is used by default
-Added onEndScreen() method to Controller class since it had already onStartScreen()

[bgroenks96]

Looks pretty nifty 😉 (stealing your lines @void256 )

Since this has some changes in core Nifty code, I'll need to run some regression tests to make sure none of the new controller factory code causes any issues.

Once that all checks out, I'll go ahead and merge it. It looks good!

[BorealFeast]

I hope this will help! And be merged soon ! :)

[bgroenks96]

Ok well I can't get the examples to run on my current system for some reason (some JOGL/LWJGL classpath issue that I don't feel like spending time on)...

But all of the modules compiled and built with 100% test pass-rate. So that's good enough for me! 👍

[BorealFeast]

Great!

[void256]

The commit ae267e1 broke many of the nifty-examples because they have not been updated with an implementation for the new onEndScreen() method introduced in the Controller interface!

I think this went unnoticed because some of the nifty examples still used Nifty 1.4.1 instead of the updated and correct 1.4.2-SNAPSHOT dependency. After updating to 1.4.2-SNAPSHOT this error did show up.

I'm going to fix this now though 😉

However, the mentioned commit will break all existing code that uses 1.4/1.4.1 AND the Controller interface directly (using AbstractController will be save). It will force these people to update their code when they switch to 1.4.2. I'm not sure if we really want that. Does having onEndScreen() available outweight the cost of forcing people to update their code?

Note: I'll just want to raise awareness that changing core Nifty interfaces might have a bigger impact then originally expected...

[bgroenks96]

However, the mentioned commit will break all existing code that uses 1.4/1.4.1 AND the Controller interface directly (using AbstractController will be save). It will force these people to update their code when they switch to 1.4.2. I'm not sure if we really want that. Does having onEndScreen() available outweight the cost of forcing people to update their code?

Yeah that's definitely a really good point. We should probably reconsider that change.

As for the examples being broken, I failed to check those for this PR because I was having issues getting them to run at the time. That's why I didn't see that.

[bgroenks96]

@void256 Come to think of it... I can't really think of any reason for Controller to need onEndScreen by default anyway. It seems like most Controller implementations shouldn't really care too much about it.

Maybe we should just revert that commit but not 4c5c990?

[void256]

@bgroenks96 you couldn't see the problem even if you could get the examples to work tho. When the maven release plugin updated the dependencies from 1.4.1 to 1.4.2-SNAPSHOT it didn't update the properties in the nifty-examples pom.xml so they still happily used Nifty 1.4.1 the whole time which didn't had that controller change. They would happily compile using the old Nifty 😉

And I only saw it because I've changed the groupId for our move to sonatype and the central and it didn't compile for me because I initially disabled the old maven repo at sf.net and therefore it couldn't find 1.4.1 😅

About reverting it - I'm really not sure to be honest. I can think of use cases where it could be useful. However, we did live without it for quite some time now and no one seemed to miss it that much (since I didn't hear any screaming regarding it yet 😉)

If @BorealFeast just added it because he thinks it was missing and he doesn't use it at all in his code I'd vote to revert the commit. Simply because there seems to be no actual need for it and we lived long without that method and it introduces a compile-time error if existing 1.4.x code uses the Controller interface directly (which is very likely I think).

[BorealFeast]

To be honest, I'm actively using the onStartScreen and onEndScreen life cycle of the controllers. This helped me to decouple my architecture where a controller can now be standalone. Well, these are just for my use cases.
On the other hand, I've heard (read) quite few times that people were trying to have multiple Screens active at the same time, which is not intended to be done by nifty. Having Controllers with their life cycle that looks like the one of the ScreenControllers might be a good alternative for this need. And it could be useful to reuse code, like the following xml:

   <screen id="main">
        <layer>
            <control name="mainMenu"/> <!-- main menu ui -->
        </layer> 
    </screen>

    <screen id="game">
        <layer>
            <control name="gameUI"/> <!-- game ui -->
        </layer>
        <layer>
            <control name="mainmenu" visible="false" /> <!-- main menu ui overlay -->
        </layer>    
    </screen>

[void256]

Alright! Thanks for the reply @BorealFeast .. then we'll keep things as they are.

[BorealFeast]

Great!