Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: migrate to gestaltv7 #4593

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e55b7f0
feature: migrate to gestaltv7
pollend Mar 21, 2021
f9ce52f
bugfix: correct copy strategy
pollend Mar 28, 2021
46c29ee
chore: remove copy strategy
pollend Mar 28, 2021
5504449
fix formatting
pollend Mar 28, 2021
9afe704
refactor(handleSwitchToGameEnvironment): these are all one copy strategy
keturn Apr 1, 2021
850ee40
build: no dependency on gestalt-entity-system yet
keturn Apr 1, 2021
911110a
Merge branch 'nui-gestalt-separation' into feature/migrate-gestalt-v7
keturn Apr 2, 2021
d9af0ba
chore(ComponentSystem): debugging logs
keturn Apr 2, 2021
0a175af
chore(UISkinWithUrn): update for gestalt 7
keturn Apr 2, 2021
8e26159
chore: revert move of jmh sources
keturn Apr 2, 2021
7b1bd17
fixup! chore: revert move of jmh sources
keturn Apr 2, 2021
cba8ef5
core(EntitySystemSetup): null checking
keturn Apr 2, 2021
3986c4d
refactor(ModuleManager): restore some organization from the develop b…
keturn Apr 2, 2021
b874418
test: reduce duplicate exception logs and wrapping
keturn Apr 2, 2021
bfb1a8e
test: add mockito-inline to empower mockito to do more invasive things
keturn Apr 2, 2021
6412aab
test(ModuleEnvironmentTest): assert, don't `assume`!
keturn Apr 2, 2021
a7e2fb3
test(TerasologyTestingEnvironment): avoid NPE on tearDown (the result…
keturn Apr 2, 2021
f3711c3
test(InputSystemTest): do not always include TestEventButton in the r…
keturn Apr 2, 2021
442d2fd
feat(ModuleManager): add method for creating an environment with reso…
keturn Apr 2, 2021
31715d2
test(BindsSubsystemTest): update for gestalt-v7
keturn Apr 2, 2021
1c48c86
test(TypeSerializerTest): lint
keturn Apr 2, 2021
42ffb8e
chore: bump version to 4.4.1-SNAPSHOT
keturn Apr 2, 2021
2f9d4af
Merge remote-tracking branch 'origin/nui-gestalt-separation' into fea…
keturn Apr 2, 2021
20546f1
fix(ModuleManager): loosen auto-generated dependency version
keturn Apr 2, 2021
c69a87f
feature: migrate gestalt v7 discord subsystem (#4613)
pollend Apr 4, 2021
d8de0d3
Merge remote-tracking branch 'origin/develop' into feature/migrate-ge…
pollend Apr 4, 2021
3c5ff6e
bugfix: correct newGameScreen
pollend Apr 4, 2021
873dec8
move discord to engine namespace
pollend Apr 4, 2021
ccec83d
Merge branch 'develop' into feature/migrate-gestalt-v7
pollend Apr 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,32 @@ public ModuleManager(String masterServerAddress, List<Class<?>> classesOnClasspa

registry = new TableModuleRegistry();

engineModule = loadEngineModule(classesOnClasspathsToAddToEngine);
engineModule = moduleFactory.createPackageModule("org.terasology.engine");
Module nui = createSyntheticPackageModule("nui", "3.0.0", "org.terasology.nui");
Module discordSubsystem = moduleFactory.createPackageModule("org.terasology.subsystem.discordrpc");

loadModulesFromApplicationPath(pathManager);
createDependency(engineModule, nui);

registry.add(nui);
registry.add(engineModule);
registry.add(discordSubsystem);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrmmm. Not sure about adding hardcoded references to subsystems in engine's ModuleManager.

The point of extracting subsystems—as I understand it—is to make it so the engine doesn't always need to depend on them. i.e. a facade should be able to start an engine that doesn't have a DiscordRPC subsystem at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, at the moment its a hack to get the game to run. System is registered but it can't resolve autoconfig when it tries to scan with environment.


loadModulesFromApplicationPath(pathManager);
ensureModulesDependOnEngine();

setupSandbox();
loadEnvironment(Sets.newHashSet(engineModule), true);
loadEnvironment(Sets.newHashSet(engineModule, nui, discordSubsystem), true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: We should switch this to the new resolveAndLoadEnvironment method. It'll do dependency resolution so we don't have to manually keep these lists in sync with every change.

installManager = new ModuleInstallManager(this, masterServerAddress);
}

private void createDependency(Module parent, Module child) {
DependencyInfo dependencyInfo = new DependencyInfo();
dependencyInfo.setId(child.getId());
dependencyInfo.setMinVersion(child.getVersion());
dependencyInfo.setMaxVersion(child.getVersion().getNextPatchVersion());
parent.getMetadata().getDependencies().add(dependencyInfo);
Comment on lines +92 to +97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 makes sense to have a function for this.

TODO: refactor ensureModulesDependOnEngine method (just below this one) to use this.

FIXME: note 20546f1 — getNextPatchVersion may break for this purpose when base version is a SNAPSHOT.

}

/**
* I wondered why this is important, and found MovingBlocks/Terasology#1450.
* It's not a worry that the engine module wouldn't be loaded without it.
Expand All @@ -110,27 +125,6 @@ private void loadModulesFromApplicationPath(PathManager pathManager) {
scanner.scan(registry, paths);
}

private Module loadEngineModule(List<Class<?>> classesOnClasspathsToAddToEngine) {
// FIXME: is `classes…toAddToEngine` gone? Did we ever use it in the first place?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check for classesOnClasspathsToAddToEngine usage? I know it's not usually used, so maybe this is a non-issue, but I had a vague memory about it being used somewhere...

Module engine = moduleFactory.createPackageModule("org.terasology.engine");
// TODO: document why nui is a module when other libraries are not
Module nui = createSyntheticPackageModule("nui", "3.0.0" /* FIXME */, "org.terasology.nui");

DependencyInfo nuiDependency = new DependencyInfo();
nuiDependency.setId(nui.getId());
nuiDependency.setMinVersion(nui.getVersion());
nuiDependency.setMaxVersion(nui.getVersion().getNextPatchVersion());
engine.getMetadata().getDependencies().add(nuiDependency);

registry.add(nui);
registry.add(engine);

// FIXME: why doesn't gestalt-v7 need this?
// enrichReflectionsWithSubsystems(engineModule);

return engine;
}

public ModuleManager(Config config) {
this(config, Collections.emptyList());
}
Expand Down Expand Up @@ -242,19 +236,4 @@ public ModuleFactory getModuleFactory() {
return moduleFactory;
}

// private void enrichReflectionsWithSubsystems(Module engineModule) {
// Serializer serializer = new XmlSerializer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why doesn't the gestalt-v7 branch need an enrichReflectionsWithSubsystems method? v7 hasn't done away with Reflections yet…

// try {
// Enumeration<URL> urls = ModuleManager.class.getClassLoader().getResources("reflections.cache");
// while (urls.hasMoreElements()) {
// URL url = urls.nextElement();
// if (url.getPath().contains("subsystem")) {
// Reflections subsystemReflections = serializer.read(url.openStream());
// engineModule.getReflectionsFragment().merge(subsystemReflections);
// }
// }
// } catch (IOException e) {
// logger.error("Cannot enrich engine's reflections with subsystems");
// }
// }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"id" : "discord",
"version" : "4.4.1-SNAPSHOT",
"displayName" : "Discord Subsystem",
"description" : "Discord Subsystem"
}