feature: migrate to gestaltv7 #4593
Conversation
|
any clue why this isn't working |
| // umm need for android dependencies | ||
| repositories { | ||
| google() | ||
| } | ||
|
|
There was a problem hiding this comment.
I have these littered around in a few places any clue how to correct these imports. looks like we are dependencies on some android dependencies ?!
|
Adding it to terasology-repositories in build-logic would normally be sufficient for adding a repository, but since it's a dependency of gestalt and build-logic uses gestalt itself, I think it'll also need to be in (Yeah, I'm starting to doubt the wisdom of making build-logic depend on gestalt. Or wonder if there's some way for build-logic to be responsible for its own repositories so it doesn't leak out to the other |
engine-tests/src/test/java/org/terasology/documentation/BindingScraper.java
Outdated
Show resolved
Hide resolved
I was referencing @DarkWeird older PR. I'll undo that change. |
|
I'm having some strange rendering problems where you only see the last dropped item. not sure what is causing it and how to resolve the problem. |
Dropped items has same Probably it can be resolved with java 8 (reflections have several glitches at java 11... classloadersss...) |
|
If this is using MovingBlocks/TeraNUI#46, does that mean we don't need to do the change to the NUI namespaces in MovingBlocks/TeraNUI#27 at the same time? edit: oh, wait, that's what DW's comment just prior to this one was about, things failing because a nui package is not part of |
I think use BSA's PR firsts, then provide support in nui-gestalv7 module is better then syntetic module |
keturn
added this to In progress
in gestalt v7 migration (module & asset)
via automation
|
Sounds like you've been learning about what works and what doesn't. We are going to end up with a PR that changes a zillion import statements; with changing the packages of things like The thing with Are there other fixes that would also apply to the current code even without the gestalt change? e.g. the camera position? |
pollend
force-pushed
the
feature/migrate-gestalt-v7
branch
from
c83bac4
to
6854c95
Compare
pollend
force-pushed
the
feature/migrate-gestalt-v7
branch
from
6854c95
to
e55b7f0
Compare
|
I haven't tried it out yet, but I strongly suspect the thing you described with Terasology/engine/build.gradle Lines 130 to 134 in 21d13a3 note how the comment says "all sourceSets." Which might be useful if we had parts of engine written in Kotlin or Scala, but is probably not helping us as it is. And yes, I'm still hoping to roll back the jmh part of this PR. Not just for size; also because I think in general we want to move in the direction of "move things from engine-test/src/test to engine/src/test" instead of "move things from engine to engine-test." Admittedly I am really not at all clear on where the |
yep, go ahead and revert the jmh part of this PR. |
|
Getting two NPEs after I save a screen shot. One of them is coming from NUISkinEditorScreen, which is very puzzling given that I don't have that screen open nor have I used it this run at all: |
|
The other is from DebugControlSystem: [I really wish java or github or something in this toolchain would inline sources in tracebacks.] We're not on Java 14 yet so we don't have useful NPE messages, but that line is so I guess it's probably |
|
* feature: add subsystem to modulemanager * bugfix: correct crash with ModuleManager
| private Module loadEngineModule(List<Class<?>> classesOnClasspathsToAddToEngine) { | ||
| // FIXME: is `classes…toAddToEngine` gone? Did we ever use it in the first place? |
There was a problem hiding this comment.
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...
| // private void enrichReflectionsWithSubsystems(Module engineModule) { | ||
| // Serializer serializer = new XmlSerializer(); |
There was a problem hiding this comment.
why doesn't the gestalt-v7 branch need an enrichReflectionsWithSubsystems method? v7 hasn't done away with Reflections yet…
| 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); |
There was a problem hiding this comment.
👍 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.
|
|
||
| registry.add(nui); | ||
| registry.add(engineModule); | ||
| registry.add(discordSubsystem); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| ensureModulesDependOnEngine(); | ||
|
|
||
| setupSandbox(); | ||
| loadEnvironment(Sets.newHashSet(engineModule), true); | ||
| loadEnvironment(Sets.newHashSet(engineModule, nui, discordSubsystem), true); |
There was a problem hiding this comment.
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.
|
Build #25 (20546f1) had 2 failing tests: http://jenkins.terasology.io/teraorg/job/Terasology/job/engine/job/PR-4593/25/ it has now regressed to 43 failures. and PlayerConfig-to-AutoConfig was inculded in build 25, so that wasn't the problem. |
|
I decided to move the discord package since trying to register it to the environment is very difficult and harder to maintain if we introduce more subsystems. @keturn |
|
I went back a few steps to see if I could reproduce the troubles you were having with DiscordRPC. and indeed I did! so I'm not sure how I had things working as well as I remember; maybe they worked until I merged in the AutoConfig changes from develop, and I didn't re-check after that? That's my best guess. I also discovered that we seem to have the modules using the JsonSerializer for Reflections but engine and subsystems are still on XML, which is sure to be a source of confusion. |
|
I plan to experiment a little to see if we can work things without sacrificing the engine/subsystem separation. The plan is something like:
That branch can be found at #4622 |
|
@pollend Which thing was it that called for the |
|
uuuh, I don't remember why
|
|
superseded by #4622 |
Here is the initial migration to gestalt-v7.
Changes
Issues
google()repositorychore/gestalt-v7-migrationModule Changes