MovingBlocks / Terasology

Terasology - open source voxel world
http://terasology.org
Apache License 2.0
3.67k stars 1.33k forks source link

Split engine to subsystems #4304

Open DarkWeird opened 3 years ago

DarkWeird commented 3 years ago

Extraction subsystems should:

  1. simplify codebase via extracting common components
  2. make some components optional/replaceable. e.g: Optional: Telemetry and Discord not needs for server build Replaceable: AudioLwjgl and AudioNull subsystems.
  3. provide native library support for modules. e.g: Kallisti -> Jnlua CoreRendering -> GraphicsLwjgl

Extraction subsystems shouldn't:

  1. Replace Modules
  2. Downloads from server or internet
  3. Loads dynamically

Draft extraction scheme:

Disclaimer: some listed subsystem is just library - haven't EngineSubsystem implementation. but i cannot place it at libs directory, because libs directory used for external libraries (external repo)

Subscribables is timed subsystem. while AutoConfigs will not extracted and configs not migrated to autoconfigs

Required Features:

  1. 4244 - for autoconfig. it ideally config system for subsystem ecosystem. needs 7 and 2.

  2. Module -> Subsystem dependency. Partial current solution for 'Library' subsystems - engine provides them as gradle's api dependency (formal it is engine dependency)

Extractions

  1. DiscordRPC subsystem #4233
  2. TypeHandlerLibrary #4255
  3. ✅ Add generic Serializer for TypeHandlerLibrary #4324
  4. extract EntitySystem. ( requires ✅ #4565 for pure events and ✅ #4324 for serialize components)
  5. protobuf serialize over GenericSerilaizer(3)
  6. network subsystem - needs 4
  7. GsonSerializer #4270 - needs 2.( follow-ups - migrate engine's code from pure Gson to GsonSerializer) Outdated with #4324
  8. extractTypeWidgetLibrary -- but first needs to merge #4244 ✅
  9. extract AutoConfigs - needs 7.
  10. extract Audio(Null|API|Lwjgl) subsystems. required .
  11. extract Graphics - required AutoConfigs as subsystem and migrated Rendering configs to AutoConfig.
  12. extract Record&Replace - highly integrated in engine. needs refactor WorldStorage to make it extendable.
  13. ???
  14. PROFIT!
jdrueckert commented 3 years ago

In Discord @DarkWeird mentioned the following configs that will need to be migrated to AutoConfig:

Cervator commented 3 years ago

Small note:

Optional: Telemetry and Discord not needs for server build

Discord: correct. Telemetry: want! Great way to send server logs and performance. There are client specific bits to the telemetry system though, like blocks broken and such. That part we could certainly skip, but then I'm not sure if that would enable by default 🤔

DarkWeird commented 2 years ago

Investigation of possible to extract AutoConifg as Subsystem:

  1. Context should be extracted (#4930) - AutoConfig injects Autoconfigs to context and have @In Injects
  2. AutoConfigScreen - takes ModuleManager(engine) and CoreScreenLayer(engine) 3.AutoConfigWidgetFactory - takes ModuleManager(engine) and TranslationSystem(engine)
  3. ColorConstraintWidgetFactory - takes CieCamColors(engine, used for ColorWidget and PlayerConfig also useless ColorHueAnimator), TextureUtil(engine, for color uri) and Texture(engine, loading asset)
  4. LocaleConstraintWidgetFactory - takes SimpleUri(engine, can be replaced with ResourceUrn(gestalt)) and TranslationSystem(engine)
  5. AutoConfig - takes SimpleUri(engine, can be replaced with ResourceUrn(gestalt)
  6. AutoConfigManager - takes Context(engine, subsystems:Context on the way), SimpleUri(engine), PathManager(engine, for save and loading), ModuleManager(engine, can be replaced with directly ModuleEnvironment(gestalt-module)) and ReflectionUtil(engine, resolve SimpleUri from config class and ModuleEnvironment(module-gestalt)))

Possible bonus. if we extract saving and loading from AutoConfigManager then per-game-saves will easier.(PathManager)

Think to do:

  1. extract Context (#4930)
  2. extract TranslationSystem (there have Translator(nui). but we needs getProject method.. which don't in traslator) or do TODO there
  3. replaces SimpleUri with ResourceUrn (partially)
  4. idk what to do with CieCamColors(engine)
  5. idk what to do with Texture and TextureUtil
  6. Idk where move ReflectionUtil.getFullyQualifiedSimpleUriFor(engine). because AutoConfig is single user... but Yet Another ReflectionUtl.... (we have it in TS, in NUI)
keturn commented 2 years ago

I was having trouble understanding #4930, but maybe re-reading the last comment here is helping.

I was thinking: Why bother extracting Context? Perhaps it is so Subsystems can use it, but, no, Subsystems do depend on Engine — they must, if for no other reason than the EngineSubsystem interface is defined in Engine. That means Subsystems may use a Context class defined in Engine too.

Looking at this, it seems it's because you want to extract AutoConfig as a library that has no Engine dependency?

It would help me keep things straight if we use different words for Subsystems vs non-Engine libraries. If the only obstacle to calling those “libraries” or “libs” is that we currently have the /libs/ directory doing a different job in our Gradle configuration, we can surely move or rename that.

I am also not totally sold on the utility of extracting libs to their own gradle sub-projects. It does carry some maintenance cost with it: for example, the recent case where we neglected to run tests on TypeHandlerLibrary for many months because we no longer invoke tests in the same way in all our sub-projects. On the other hand, it does add some constraints that help keep things more organized by preventing circular dependencies between packages.

DarkWeird commented 2 years ago

I don't like current TS's engine:

  1. A big mess of classes and packages, which connected somehow, leaked interfaces
  2. Poor submodule level documentation
  3. Mix from bootstrap code(states, subsystems, TerasologyEngine...) and engine module.
  4. Many conditional code, which don't using 99% time (e.g. telemetry, record and replay, headless impls(server or mte users)

Then i suggest:

  1. Split to small code modules with strict describes, what this code module does.
  2. Document subsystem and parts as code module (see THL. It pretty. Isn't it?)
  3. Separate engine module. Bootstrap should be splitted by (1)
  4. Conditional code should be as separate code module. (Summary: made enterprise level splitting (api/impl): Like androids apps, like springs apps. Like bestpractics about code modules sinse ejb)

I think about those categories now:

  1. External lib (/libs/) - library, which can be used everywhere. For example: You can clone guava there
  2. EngineSubsystem (/subsystems/) - engine subsystem implementation. 2.1. provides functionality for modules (there should be like jnlua bindings, don't should break sandbox, cannot be downloaded, related modules cannot be activated without this subsystem in game installation) 2.2. provide functionality for facades (there DiscordRPC, using engine data, passtru it to discord, can be using by facade only. Circular dep otherwise)
  3. Engine-part or engine-lib - provides functionality or api(!) for other subsystem(or modules, or facades) 3.1 candidate for being lib - there THL, it almost ready for being gestalt-serialization. 3.2. api - interfaces which can be used by subsystem or engine(bootstrap part) - there Context(implementation in engine), there will NetworkAPI (provides server/client interfaces, implementation will at NetworkNetty)

P.S. AutoConfig should provide config functionality for:

  1. Modules (not implemented yet, except engine module.it not ordinal module)
  2. Engine Subsystems (like lwjgl or discord)

We cannot provide functionality for non-extracted subsystem and engine module... Because autoconfig using context. But we can extract Context-api and then extract autoconfigs!

Another example: We cannot extract rendering. We should extract api and impl... Because rendering using from bootstrap code(nuimanager) Then. Api should not use engine(circular dep) .

MTE example: MTE deeply using engines parts(runs engines and other infrastructure) We can extract it as subsystem (code module) and use it at engine.test(source set), facades, modules.tests. I think ideally place :3

You can provide you naming for /subsystem/

keturn commented 2 years ago

I think these are good categorizations. :+1:

Which of them want to be their own gradle-(sub)projects?

Advantages of being a separate gradle (sub)project:

Disadvantages of being separate:

(There are, of course, ways to mitigate these disadvantages, but it adds a bit of work to do so.)

If an EngineSubsystem is optional or replaceable, so that a particular Facade may choose to build an application without it, I think that provides a good reason for wanting a separate jar.

On the other hand, a library such as THL will necessarily be a dependency for everything; no one will ever get the engine.jar without also getting the THL.jar.

Another thought, now that Terasology requires Java 11: Are Java Modules an alternative (better? worse?) way of describing the connections between these different packages and enforcing the API encapsulations we want for them? (If that is a can of worms that is too messy for now, please feel free to say so.)

DarkWeird commented 2 years ago

Each has its own build.gradle to maintain

Yeah. I think about version catalog :3

More difficult to navigate the HTML Javadoc

Ehm. We have this? I think somewhere exists solution with html javadocs over several java libs.

On the other hand, a library such as THL will necessarily be a dependency for everything; no one will ever get the engine.jar without also getting the THL.jar.

There another reasons:

  1. Smaller engine part > easier maintain, stricter dep describing.
  2. (Yeah it can be convert to java module easier :p)

Another thought, now that Terasology requires Java 11: Are Java Modules an alternative ... I have some thoughts about this, but it is future:

  1. Replaces @API and our sandbox/security
  2. Activate more aggressive optimization
  3. Seems easier tera module describes.(instead package module and jar module Cons:
  4. Java module have problems with unit tests yet(unit tests cannot access to internal packages)
  5. Needs support from gestalt side
keturn commented 2 years ago

Using Java Modules as gestalt Modules is a whole topic in itself. For now, I was wondering about these library-ish things, whether java modules' ability to define requirements and exports would help in this… but taking another look at that now, I think jars can only contain one module, and each gradle SourceSet can make one jar/module. That means this is not an alternative we could do instead of splitting these into different source directories— it's something additional we could do after splitting them up.

keturn commented 2 years ago

Mainly though, the "smaller engine part > easier maintenance" is what I am slow to understand. Why is org/terasology/engine/registry/*.java easier to maintain if it is in /subsystems/Context/src/main instead of /engine/src/main?

You mention dependencies, and it does make it so if you're editing something there you can't just auto-import something from org.terasology.engine.world without thinking about it; you have to pause and consider whether to manually add that dependency.

That is worth something, but I keep wondering if I have missed something else, because it doesn't quite sell me by itself.

keturn commented 2 years ago

On Javadoc: We do have Javadoc published on jenkins, if nowhere else. I think the build generates javadoc for everything, but the Jenkins built-in javadoc integration only publishes one javadoc directory for the project, so we have it pointing at /engine currently.

DarkWeird commented 2 years ago

Mainly though, the "smaller engine part > easier maintenance" is what I am slow to understand. Why is org/terasology/engine/registry/*.java easier to maintain if it is in /subsystems/Context/src/main instead of /engine/src/main?

Try to locate all parts of rendering. Try to locate all parts of any serialization. Try to locate all parts of health and destruction. Try to locate all parts of networking.

They placed in different packages. I try isolate one functionality in one place.

Context required to extract for another subsystems. (Also... Those gradle modules should be looks fine at dependency graph visualization)