Non static way for the UI to get the requried objects from the CoreRegistry / Context instance.

[flo]

The UI classes are currently getting their data directly or indirectly via static access to the CoreRegistry.

• Widgets currently use the CoreRegistry to get an AssetManager instance. Either directly or indirectly via the Assets helper class.
• Widgets currently use the CoreRegistry to get a Keyboard instance from the InputSystem in order to check for example if shift is pressed.
• Widgets use the CoreRegistry to get other instance too like RenderableNode use it to get BehaviorNodeFactory
• When pushScreen or addOverlay of NUIManagerInternal gets called. The screen or overlay gets objects injected from the current CoreRegistry instance. Not however the weidgets of that screens or overlays. The NUIManagerInternal knows onlz the Context instance of the engine. It does not know the Context instance that is needed to perform the injection.

Suggestion 1: Add a Context argument to pushScreen and addOverlay and use it to inject the screens and overlays and all their child widgets recursivly.

Disadvantage: Widgets don't know when they get dependency injeted.

Suggestion 2 Introduce a onContextChange(Context context) in AbstractWidget, that widgets can override to for example initialize fields with assets.

Add a setContext method to UIWidget that calls onContextChange when the context changed.

This suggestion can be combined with suggestion 1.

Suggestion 3

Add Context as a constructor argument to AbstractWidget and thus all sub classes.

Widgets can then initailize their asset fields at construction.

Open Question: Shall we remove the redundant dependency injection performed for the screens/overlays? Or shall we extend pushScreen and addOvleray with an Context parameter.

Suggestion 4 Eleminate Context usage from widgets.

e.g. by extending methods like onMouseClick with a Keyboard argument, so that the shift key can be checked in the event handler.

There are bunch of open questions if we go for that.

[immortius]

I did a quick search for usages of CoreRegistry within the nui classes:

Only one widget proper (excluding screens) uses core registry - UILoadBar. It uses it to get access to the Time class to handle animation (which should possibly be handled better by the nui framework). I don't see any widgets using it to access other things.

In general I would say widgets should not be dependency injected. We should address cases where CoreRegistry is being used on a case by case basis - some may represent need for improvements to nui, some may represent a class nui should make available, some may be misuse.

Screens though do see a lot of CoreRegistry usage, and injection, because they are controllers and root containers of a portion of the UI, and tie it to world state.

In general I would suggest NUIManager should be provided with a new context during environment switches. I would say it should possible be controlled by a subsystem, and subsystems sent an event for context changes (as part of environment changes, likely). A slight complication is that a small amount of UI will continue to exist across an environment change, but this isn't likely to be a big issue (mostly loading screens, and other screens that only hook into things that also survive across context switches). I don't think it is worth giving screens an onContextSwitch event since it is nothing most screens would need to worry about (and widgets certainly shouldn't), but perhaps there could be an interface screens could implement to receive these events (and also mark them to survive context switches).

[jdrueckert]

Here's the usages (as of today) of CoreRegistry within engine/src/main/java/org/terasology/engine/rendering/nui:

skin/UISkinFormat.java:                       NUIManager nui = CoreRegistry.get(NUIManager.class);
skin/UISkinFormat.java:                       WidgetLibrary library = CoreRegistry.get(NUIManager.class).getWidgetMetadataLibrary();
layers/mainMenu/videoSettings/VideoSettingsScreen.java:        CoreRegistry.get(ShaderManager.class).recompileAllShaders();
layers/mainMenu/ReplayScreen.java:            CoreRegistry.get(GameEngine.class).changeState(new StateLoading(manifest, NetworkMode.NONE));
layers/mainMenu/NameRecordingScreen.java:     CoreRegistry.get(GameEngine.class).changeState(new StateLoading(manifest, NetworkMode.NONE));
layers/mainMenu/SelectGameScreen.java:        Optional.ofNullable(CoreRegistry.get(GameEngine.class))
layers/ingame/InspectionScreen.java:          inspectorComponent.inspectedEntity = CoreRegistry.get(EntityManager.class).getEntity(id1);
layers/ingame/DeathScreen.java:               WidgetUtil.trySubscribe(this, "exitGame", widget -> CoreRegistry.get(GameEngine.class).shutdown());
layers/ingame/metrics/NetworkStatsMode.java:  time = CoreRegistry.get(Time.class);
layers/ingame/metrics/NetworkStatsMode.java:  networkSystem = CoreRegistry.get(NetworkSystem.class);
layers/ingame/metrics/WorldRendererMode.java: return getName() + "\n" + CoreRegistry.get(WorldRenderer.class).getMetrics();
layers/ingame/metrics/WorldRendererMode.java: return CoreRegistry.get(WorldRenderer.class) != null;
layers/ingame/PauseMenu.java:                 WidgetUtil.trySubscribe(this, "mainMenu", widget -> CoreRegistry.get(GameEngine.class).changeState(new StateMainMenu()));
layers/ingame/PauseMenu.java:                 WidgetUtil.trySubscribe(this, "exit", widget -> CoreRegistry.get(GameEngine.class).shutdown());
asset/UIFormat.java:                          NUIManager nuiManager = CoreRegistry.get(NUIManager.class);
asset/UIFormat.java:                          TranslationSystem translationSystem = CoreRegistry.get(TranslationSystem.class);
asset/UIFormat.java:                          TypeHandlerLibrary library = CoreRegistry.get(TypeHandlerLibrary.class).copy();
animation/MenuAnimationSystems.java:          RenderingConfig config = CoreRegistry.get(Config.class).getRendering();
widgets/UIButtonWebBrowser.java:              Config config = CoreRegistry.get(Config.class);