Remove CoreRegistry usage: Part III + Improve ComponentSystemManager life cycle

[flo]

While the main goal of this pull request is again to reduce CoreRegistry usage it also does some noteworthy changes:

The most important one being that it changes the life cycle of the ComponentSystemManager object(Fixing #1125).

• It no longer gets added into game engine context
• It still get instanced and added to the Context of StateMainMenu and that instance still lifes for the duration of that state
• It now get instanced and added to the Context of the game during the loading phase.
• It no longer removes the objects it registered from the Context, as it stays in the context until it gets garbage collected. It is also no longer necessary to clear its list from references to other objects for the same reason.

@immortius and @emanuele3d are you fine with that solution?

This pull request also removes the delete method from the Context as it is no longer necessary. @msteiger introduced a new delete usage in #1726 but the lack of it wasn't an issue previously and there is surely a better way of solving it than having objects be removed from a context. e.g. like creating another sub context for the given life duration or having a manager that lifes longer.

[GooeyHub]

Refer to this link for build results (access rights to CI server needed):
http://jenkins.terasology.org/job/TerasologyPRs/96/
Hooray Jenkins reported success with all tests good!

[immortius]

Looks good to me, my only comment is perhaps ComponentSystemManager should add itself to the context in its constructor. But it is a tiny thing and is possibly clearer as it is now.

I would request it is held off until after the asset system changes are merged, but that should be soon now.

[msteiger]

About the delete usage in PreviewWorldScreen: I would assume that a new child context must be created and fed to ComponentSystemManager to have it injected into all world-gen related classes. Is that how you intended it?

[Cervator]

Big merge is done now so this should be ready to move again - but @flo may be out of pocket for a little while before this can be reviewed for conflicts

[GooeyHub]

Refer to this link for build results (access rights to CI server needed):
http://jenkins.terasology.org/job/TerasologyPRs/143/
Hooray Jenkins reported success with all tests good!

[flo]

I fixed a small merge conflict and this branch is ready for merging.

@msteiger About the PreviewWorldScreen: Sorry for not replying earlier. I am not sure if you noticed but Immortius changed the PreviewWorldScreen in the asset integration branch to create a sub context and assign it temporarly to the CoreRegistry as global context, so that code that still uses the CoreRegistry remains working until all CoreRegistry usages got removed. So for the mean time everything is okay.

AnotherWorld preview seems broken(not related to this pull quest), since the world field of the world generator is null when the FacetLayerPreview gets created in WorldPreviewScreen#onOpened:

                worldGenerator = worldGeneratorManager.searchForWorldGenerator(worldGenUri, environment);
                worldGenerator.setWorldSeed(seed.getText());
                previewGen = new FacetLayerPreview(environment, worldGenerator);

@msteiger I am not familiar with world generation code but maybe a initialize call is missing? Or should all world generators have a lazy version of getWorld?

That issue asside I however also assume that it will still continue to fail because PluggableWorldGenerator is trying to obtain the ClimateConditionsSystem from the CoreRegistry which however would mean that systems need to be activated.

@immortius, @MarcinSc, @msteiger I think we have to decide design wise if we want to allow world generators to use module specific systems for world preview generation. If we allow the use of systems we propably also want them to be ready which means that the system initialization has to happen at latest when the preview screen gets opened. Depending on the complexity of the system initialization this may slow down the world preview generation.

If we want world generators to have access to the systems of the modules then we could create a direct sub context from the engine context and fill it via another ComponentSystemManager instance (Currently a sub context of StateMainMenut gets used that has a few basic systems initialized).

[Cervator]

Will aim to merge this tomorrow sometime if some time to do so appears, if no other obstacles :-) Or somebody else can test and merge.

The world gen / preview discussion sounds worthy of an issue to work next sometime. It does sound a bit like the preview would fit well with the new sub context thing? Maybe it could disable some of the time consuming stuff (if there is anything that sticks out and isn't needed)

[immortius]

I think we need to consider any way to possibly simplify the world previewing - what is the minimum needed to generate the preview. Are assets needed? Potentially (although I would argue that blocks should not be needed). The entity system? Quite likely (too much hangs off of it). Everything? Or conversely should world previews be handled through a complete environment switch and load rather than messing around with a partial switch?

This actually touches on the concept of gametype again, as we would then have two reasons to load a particular module set - to play a game, and to preview a world. Essentially previewing the world is a playerless, worldless gametype that would run instead of other game types.

But really I think that is trying too hard. I would suggest instead we require world generators to optionally provide a preview that must be generated sans component systems, and possibly without additional assets. If they cannot the preview would be marked as unavailable (but configuration options still provided).

[MarcinSc]

On the point of PluggableWorldGenerator using ClimateConditionsSystem, it seems we are currently unable to pass any information from world generator to systems if they need to share some information. Neither WorldGenerator should have access to Systems, nor Systems have access to Facets (or generator). I'm pretty sure we should have some way of doing this and I'm open to suggestions. Not all the information about the world generation can be conveyed in blocks, even world generator having an ability to create entities would help, as the information (parameters) could be passed via them.

To remove the "CoreRegistry.get()" from PluggableWorldGenerator we can temporarily duplicate the code that is in ClimateConditionsSystem that establishes base climate conditions and make all the settings into constants.

[msteiger]

Some world gens. use assets: HeightMapWorldGenerator uses textures to derive the height map and potentially other 2D information. I'd also like to use the pojo prefabs from NameGenerator and draw labels for islands, towns, etc. on the preview map.

I thought about loading BlockManager for WorldViewer, but it comes with a lot of dependencies, so I used a dummy block manager that returns AIR on all requests to avoid NPE crashes instead. I believe that blocks and entities/components should not be necessary for a preview.

How about reorganizing the UI screen and making the preview/config screen the final screen before switching to in-game? Then, the module env. could be fully switched and directly re-used for the game. Maybe even the generated facets could be re-used.

@MarcinSc I think that creating entities in the world gen. is the most versatile approach. I plan to have a TownGeneratorFacetProvider that also creates both facet and entity for each town. Maybe the entity could hold a reference to the facet? Not sure yet.

In any case, this is beyond this PR imho.

[Cervator]

Went ahead and merged this since it was discussed earlier, and split out a separate issue for the world preview discussion.

Tested out fine in something like TTA, although JoshariasSurvival crashes on placing a workbench (#1779) - unsure if that relates to this PR or not. We're unstable and backed up with PRs so moving forward anyway :-)

@Josharias - ping!