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

Merge pull request #3589 from Adrijaned/loadProcessDoc #3589

Merged
merged 8 commits into from Jan 18, 2020

Conversation

@Adrijaned
Copy link
Member

Adrijaned commented Dec 25, 2018

Contains

removed NOP loadProcesses, docced implementors of StepBasedLoadProcess + the class itself, nothing implements directly LoadProcess anymore

How to test

Test nothing broke

Outstanding before merging

None of these is blocker, so might as well be left for another PR to make this one easily testable and reviewable

  • javadoc for lots of loadProcesses still missing
  • other loadProcesses might still be NOP, yet present
@GooeyHub

This comment has been minimized.

Copy link
Member

GooeyHub commented Dec 25, 2018

Hooray Jenkins reported success with all tests good!

@Cervator

This comment has been minimized.

Copy link
Member

Cervator commented Jan 11, 2019

Can confirm this doesn't cause any syntax errors, even in a mega-workspace with the server facade present. Nice!

However, it worries me a bit that those three classes, CacheBlocks, CacheTextures, and LoadPrefabs get tossed - they might not appear to have a direct effect, but they might prepare some sort of state some things depend on?

I did hit a few issues, although I'm not totally sure if they're legit or just quirky. Specifically I keep hitting NPEs involving org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183) as if something hasn't cached or loaded. None of the unit tests in the ModuleTestingEnvironment or in the TestReplayModule work (but I think some of those may fail already) despite all the engine tests being OK. I can't run the game with WildAnimals which involve behavior trees. Although at the same time I get the same error even just with the BlockPicker module and Core enabled (wondering about the cache blocks bit)

Oddly I can start a headless server via the server facade just fine. It might involve a player actually starting in a world causing chunks to generate or other init code of some sort?

Some fuller snippets follow:


ModuleTestingEnvironment

00:18:58.013 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab engine:succeed with inputs [/engine/assets/prefabs/behaviorActions/succeed.prefab]
00:18:58.014 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...
00:18:58.033 [main] ERROR org.terasology.config.Config - Failed to save config
java.nio.file.FileAlreadyExistsException: /config.cfg
	at org.jboss.shrinkwrap.impl.nio.file.ShrinkWrapFileSystemProvider.newByteChannel(ShrinkWrapFileSystemProvider.java:279)
	at java.nio.file.spi.FileSystemProvider.newOutputStream(FileSystemProvider.java:434)
	at java.nio.file.Files.newOutputStream(Files.java:216)
	at java.nio.file.Files.newBufferedWriter(Files.java:2860)
	at org.terasology.config.Config.save(Config.java:159)
	at org.terasology.engine.subsystem.common.ConfigurationSubsystem.shutdown(ConfigurationSubsystem.java:127)
	at org.terasology.engine.TerasologyEngine.cleanup(TerasologyEngine.java:500)
	at java.util.ArrayList.forEach(ArrayList.java:1249)
	at org.terasology.moduletestingenvironment.ModuleTestingEnvironment.tearDown(ModuleTestingEnvironment.java:122)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:33)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

java.lang.NullPointerException
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshPrefabs(BehaviorNodeFactory.java:177)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshLibrary(BehaviorNodeFactory.java:124)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.postBegin(BehaviorNodeFactory.java:89)
	at org.terasology.engine.modes.loadProcesses.PostBeginSystems.step(PostBeginSystems.java:47)
	at org.terasology.engine.modes.StateLoading.update(StateLoading.java:234)
	at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:458)
	at org.terasology.moduletestingenvironment.ModuleTestingEnvironment.createHost(ModuleTestingEnvironment.java:268)
	at org.terasology.moduletestingenvironment.ModuleTestingEnvironment.setup(ModuleTestingEnvironment.java:113)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)


TestReplayModule




00:15:57.918 [Thread-0] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab engine:sequence with inputs [/engine/assets/prefabs/behaviorActions/sequence.prefab]
00:15:57.919 [Thread-0] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab engine:succeed with inputs [/engine/assets/prefabs/behaviorActions/succeed.prefab]
Exception in thread "Thread-0" java.lang.RuntimeException: java.lang.NullPointerException
	at org.terasology.replayTests.CuttingGrassBlockPositionReplayTest.lambda$new$0(CuttingGrassBlockPositionReplayTest.java:39)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshPrefabs(BehaviorNodeFactory.java:177)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshLibrary(BehaviorNodeFactory.java:124)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.postBegin(BehaviorNodeFactory.java:89)
	at org.terasology.engine.modes.loadProcesses.PostBeginSystems.step(PostBeginSystems.java:47)
	at org.terasology.engine.modes.StateLoading.update(StateLoading.java:234)
	at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:458)
	at org.terasology.ReplayTestingEnvironment.mainLoop(ReplayTestingEnvironment.java:168)
	at org.terasology.ReplayTestingEnvironment.openReplay(ReplayTestingEnvironment.java:141)
	at org.terasology.replayTests.CuttingGrassBlockPositionReplayTest.lambda$new$0(CuttingGrassBlockPositionReplayTest.java:37)
	... 1 more


Running game with WildAnimalsGenome (uses behavior trees):


00:21:59.872 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab engine:sequence with inputs [/engine/assets/prefabs/behaviorActions/sequence.prefab]
00:21:59.873 [main] INFO  o.t.e.prefab.internal.PrefabFormat - Attempting to deserialize prefab engine:succeed with inputs [/engine/assets/prefabs/behaviorActions/succeed.prefab]
00:21:59.877 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.lang.NullPointerException: null
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshPrefabs(BehaviorNodeFactory.java:177)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshLibrary(BehaviorNodeFactory.java:124)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.postBegin(BehaviorNodeFactory.java:89)
	at org.terasology.engine.modes.loadProcesses.PostBeginSystems.step(PostBeginSystems.java:47)
	at org.terasology.engine.modes.StateLoading.update(StateLoading.java:234)
	at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:458)
	at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:421)
	at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:397)
	at org.terasology.engine.Terasology.main(Terasology.java:156)
00:21:59.878 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...
00:21:59.908 [Thread-5] INFO  o.t.e.s.rpc.DiscordRPCSubSystem - Discord RPC >> Disconnected!
java.lang.NullPointerException
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshPrefabs(BehaviorNodeFactory.java:177)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshLibrary(BehaviorNodeFactory.java:124)
	at org.terasology.logic.behavior.nui.BehaviorNodeFactory.postBegin(BehaviorNodeFactory.java:89)
	at org.terasology.engine.modes.loadProcesses.PostBeginSystems.step(PostBeginSystems.java:47)
	at org.terasology.engine.modes.StateLoading.update(StateLoading.java:234)
	at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:458)
	at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:421)
	at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:397)
	at org.terasology.engine.Terasology.main(Terasology.java:156)
For more details, see the log files in C:\Dev\Terasology\Git\ModuleMegaWorkspace\Terasology\logs\2019-01-11_00-20-44
@Cervator Cervator added the Api label Jan 11, 2019
@Adrijaned

This comment has been minimized.

Copy link
Member Author

Adrijaned commented Jan 15, 2019

I tried to make sure that none of the methods called by the loadProcesses had any side-effects - maybe I missed something?

On rechecking the CacheBlocks still looks completely side-effect free

@Adrijaned

This comment has been minimized.

Copy link
Member Author

Adrijaned commented Jan 15, 2019

On not-completely-related-but-somewhat note - what about @Contract annotations? Would these be viable for Tera?

@jellysnake

This comment has been minimized.

Copy link
Contributor

jellysnake commented Jan 22, 2019

What are you referring to when you mean @Contract
If you mean Design by Contract then we already have a few methods that use @Preconditions via some library by Google

@Adrijaned

This comment has been minimized.

Copy link
Member Author

Adrijaned commented Jan 23, 2019

Jetbrains' @Contract

Copy link
Member

skaldarnar left a comment

@Adrijaned @Cervator how do we want to proceed with this? Are we sure about the files being deleted? Could they be some form of hooks which might be required later on? What were they actually doing before?

@@ -128,6 +128,7 @@ public void register(ComponentSystem object) {
context.get(EntityManager.class).getEventSystem().registerEventHandler(object);

if (initialised) {
logger.warn("System " + object.getClass().getName() + " registered post-init.");

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar May 8, 2019

Member

Why is this a warning? Should this be avoided? Why?

This comment has been minimized.

Copy link
@Adrijaned

Adrijaned May 8, 2019

Author Member

System has two methods - preInit and postInit. The javadoc for the later actually even guarantees that all systems have already been registered by the time it has been called. So, when something is registered later, that should definitely not happen

@@ -142,13 +139,10 @@ public void init(GameEngine engine) {

private void initClient() {
loadProcesses.add(new JoinServer(context, gameManifest, joinStatus));
loadProcesses.add(new CacheTextures());

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar May 8, 2019

Member

Are these processes/steps obsolete, or just not used (yet)? I would think they are/were there for a reason, but unfortunately I don't know that reason… 🙄

This comment has been minimized.

Copy link
@Adrijaned

Adrijaned May 8, 2019

Author Member

Since at least 2013, they don't do anything at all it seems to me. The code in the classes was always just moved/refitted to new interface without any change to functionality, which might well predate any piece of current framework. To figure out what those were actually meant to do, I would have to get myself a workspace from back then/find the PR with this code and hope for explaining notes

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar May 8, 2019

Member

Let's make sure that we find this PR again in case we later figure out that something breaks. Code that's doing nothing since 2013 might have some right to continue do nothing (😅) but I think we should get rid of it now 👍

@mayant15

This comment has been minimized.

Copy link
Contributor

mayant15 commented May 8, 2019

I'm getting the NPE @Cervator mentioned even when only the Core module is active. I also get it when I run a headless server from IntelliJ.

This was when I had Core, MTE and WildAnimalsGenome active, but I got the same error in all cases.

java.lang.NullPointerException
    at org.terasology.logic.behavior.nui.BehaviorNodeFactory.addToCategory(BehaviorNodeFactory.java:183)
    at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshPrefabs(BehaviorNodeFactory.java:177)
    at org.terasology.logic.behavior.nui.BehaviorNodeFactory.refreshLibrary(BehaviorNodeFactory.java:124)
    at org.terasology.logic.behavior.nui.BehaviorNodeFactory.postBegin(BehaviorNodeFactory.java:89)
    at org.terasology.engine.modes.loadProcesses.PostBeginSystems.step(PostBeginSystems.java:47)
    at org.terasology.engine.modes.StateLoading.update(StateLoading.java:234)
    at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:458)
    at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:421)
    at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:397)
    at org.terasology.engine.Terasology.main(Terasology.java:156)
@eviltak

This comment has been minimized.

Copy link
Member

eviltak commented Oct 16, 2019

@Adrijaned will you be able to take a look at the reported NPEs and fix this PR anytime soon?

@Adrijaned

This comment has been minimized.

Copy link
Member Author

Adrijaned commented Oct 30, 2019

@eviltak Sorry, I've kinda grown into ignoring all Github pings lately 😅 I'll try to take a look at it now.

Adrijaned added 8 commits Dec 25, 2018
I honestly believe these cachings were introduced far in history to somehow cache the assets before the current asset system. After close inspection, the code for block caching was introduced by @immortius way back in 2012 in 76ed8b0 , and remained unchanged since then. I couldn't find any side effects the code might be causing now, so removing it.
@Adrijaned Adrijaned force-pushed the Adrijaned:loadProcessDoc branch from f8380b3 to 3352d39 Oct 30, 2019
@Adrijaned

This comment has been minimized.

Copy link
Member Author

Adrijaned commented Oct 30, 2019

Alright, I figured out LoadPrefabs is still needed, due to some lazy loading functionality on some of the functions way down the abstraction tree in AssetType.

@GooeyHub

This comment has been minimized.

Copy link
Member

GooeyHub commented Oct 30, 2019

Hooray Jenkins reported success with all tests good!

@skaldarnar skaldarnar added this to the v2.3.0 milestone Jan 18, 2020
@skaldarnar skaldarnar changed the title LoadProcess documentation and small improvements Merge pull request #3589 from Adrijaned/loadProcessDoc Jan 18, 2020
@skaldarnar skaldarnar merged commit 89f8deb into MovingBlocks:develop Jan 18, 2020
2 checks passed
2 checks passed
LGTM analysis: Java No new or fixed alerts
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.