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.

Sign up for GitHub

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

fix: resolve issue when loading a world a second time #4829

Merged
merged 17 commits into from
Aug 8, 2021
Merged

fix: resolve issue when loading a world a second time #4829

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
79c0336
bugfix: resolved loading issue when attempting to start a world twice
pollend Jul 25, 2021
ff8608a
chore: cleanup
pollend Jul 25, 2021
da58e0c
chore: add InitialiseRendering
pollend Jul 25, 2021
d69a0d7
chore: add rendering to client and only how with a local client
pollend Jul 27, 2021
ca69dcf
Merge branch 'develop' into bugfix/resolve-issue-when-attempting-to-l…
pollend Jul 27, 2021
77254ed
Merge branch 'develop' of github.com:MovingBlocks/Terasology into bug…
pollend Aug 1, 2021
f8f003c
Merge branch 'develop' into bugfix/resolve-issue-when-attempting-to-l…
pollend Aug 4, 2021
c2403f4
chore: remove caching from DefaultColorSource
pollend Aug 5, 2021
029062f
Update engine/src/main/java/org/terasology/engine/core/bootstrap/Envi…
pollend Aug 7, 2021
5be727a
Update engine/src/main/java/org/terasology/engine/core/modes/StateIng…
pollend Aug 7, 2021
892bc70
Update engine/src/main/java/org/terasology/engine/core/modes/StateIng…
pollend Aug 7, 2021
d5b173f
Update engine/src/main/java/org/terasology/engine/core/modes/StateIng…
pollend Aug 7, 2021
d0a3f46
chore: apply suggestion
pollend Aug 7, 2021
9bac248
Update engine/src/main/java/org/terasology/engine/core/modes/StateIng…
pollend Aug 7, 2021
48ee146
Update engine/src/main/java/org/terasology/engine/core/modes/StateIng…
pollend Aug 7, 2021
e890e22
Update engine/src/main/java/org/terasology/engine/core/modes/loadProc…
pollend Aug 7, 2021
6d97df3
chore: add missing comment
pollend Aug 7, 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
4 changes: 1 addition & 3 deletions engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.terasology.engine.core.subsystem.common.ThreadManagerSubsystem;
import org.terasology.engine.core.subsystem.common.TimeSubsystem;
import org.terasology.engine.core.subsystem.common.WorldGenerationSubsystem;
import org.terasology.engine.core.subsystem.rendering.ModuleRenderingSubsystem;
import org.terasology.engine.entitySystem.prefab.Prefab;
import org.terasology.engine.entitySystem.prefab.internal.PojoPrefab;
import org.terasology.engine.i18n.I18nSubsystem;
Expand Down Expand Up @@ -175,7 +174,6 @@ public TerasologyEngine(TimeSubsystem timeSubsystem, Collection<EngineSubsystem>
this.allSubsystems.add(new GameSubsystem());
this.allSubsystems.add(new I18nSubsystem());
this.allSubsystems.add(new TelemetrySubSystem());
this.allSubsystems.add(new ModuleRenderingSubsystem());

// add all subsystem as engine module part. (needs for ECS classes loaded from external subsystems)
allSubsystems.stream().map(Object::getClass).forEach(this::addToClassesOnClasspathsToAddToEngine);
Expand Down Expand Up @@ -489,7 +487,6 @@ public boolean tick() {
}

Iterator<Float> updateCycles = timeSubsystem.getEngineTime().tick();
CoreRegistry.setContext(currentState.getContext());
rootContext.get(NetworkSystem.class).setContext(currentState.getContext());

for (EngineSubsystem subsystem : allSubsystems) {
Expand Down Expand Up @@ -586,6 +583,7 @@ private void switchState(GameState newState) {
if (currentState != null) {
currentState.dispose();
}
CoreRegistry.setContext(newState.getContext());
currentState = newState;
LoggingContext.setGameState(newState);
newState.init(this);
Expand Down
3 changes: 2 additions & 1 deletion engine/src/main/java/org/terasology/engine/core/bootstrap/EnvironmentSwitchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public void handleSwitchToGameEnvironment(Context context) {
context.put(EventLibrary.class, library.getEventLibrary());
context.put(ClassMetaLibrary.class, new ClassMetaLibraryImpl(context));


pollend marked this conversation as resolved.
Show resolved Hide resolved
registerComponents(componentLibrary, environment);
registerTypeHandlers(context, typeHandlerLibrary, environment);

Expand All @@ -107,7 +108,6 @@ public void handleSwitchToGameEnvironment(Context context) {
autoConfigManager.loadConfigsIn(context);

ModuleAwareAssetTypeManager assetTypeManager = context.get(ModuleAwareAssetTypeManager.class);

/*
* The registering of the prefab formats is done in this method, because it needs to be done before
* the environment of the asset manager gets changed.
Expand All @@ -116,6 +116,7 @@ public void handleSwitchToGameEnvironment(Context context) {
* existing then yet.
*/
unregisterPrefabFormats(assetTypeManager);

registeredPrefabFormat = new PrefabFormat(componentLibrary, typeHandlerLibrary);
assetTypeManager.getAssetFileDataProducer(assetTypeManager
.getAssetType(Prefab.class)
Expand Down
21 changes: 21 additions & 0 deletions engine/src/main/java/org/terasology/engine/core/modes/StateIngame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
import org.terasology.engine.core.ComponentSystemManager;
import org.terasology.engine.core.GameEngine;
import org.terasology.engine.core.GameThread;
import org.terasology.engine.core.TerasologyConstants;
import org.terasology.engine.core.bootstrap.EnvironmentSwitchHandler;
import org.terasology.engine.core.module.ModuleManager;
import org.terasology.engine.core.subsystem.DisplayDevice;
import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager;
import org.terasology.engine.entitySystem.event.internal.EventSystem;
import org.terasology.engine.entitySystem.prefab.Prefab;
import org.terasology.engine.entitySystem.systems.UpdateSubscriberSystem;
import org.terasology.engine.game.GameManifest;
import org.terasology.engine.identity.storageServiceClient.StorageServiceWorker;
Expand All @@ -28,7 +30,13 @@
import org.terasology.engine.rendering.nui.layers.mainMenu.MessagePopup;
import org.terasology.engine.rendering.world.WorldRenderer;
import org.terasology.engine.rendering.world.WorldRenderer.RenderingStage;
import org.terasology.engine.world.block.loader.BlockFamilyDefinition;
import org.terasology.engine.world.chunks.ChunkProvider;
import org.terasology.gestalt.assets.Asset;
import org.terasology.gestalt.assets.AssetType;
import org.terasology.gestalt.assets.ResourceUrn;
import org.terasology.gestalt.assets.management.AssetTypeManager;
import org.terasology.gestalt.assets.module.ModuleAwareAssetTypeManager;
import org.terasology.gestalt.module.Module;
import org.terasology.gestalt.module.ModuleEnvironment;
import org.terasology.nui.databinding.ReadOnlyBinding;
Expand Down Expand Up @@ -101,8 +109,20 @@ public Boolean get() {
@Override
public void dispose(boolean shuttingDown) {
ChunkProvider chunkProvider = context.get(ChunkProvider.class);
AssetTypeManager assetTypeManager = context.get(ModuleAwareAssetTypeManager.class);
chunkProvider.dispose();

pollend marked this conversation as resolved.
Show resolved Hide resolved
assetTypeManager.getAssetTypes().forEach(k -> {
Copy link
Member

Choose a reason for hiding this comment

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

What is k supposed to mean here? Can we please name it assetType?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assetTypeManager.getAssetTypes().forEach(k -> {
// dispose all module assets
assetTypeManager.getAssetTypes().forEach(k -> {

for (ResourceUrn urn : k.getLoadedAssetUrns()) {
if (!urn.getModuleName().equals(TerasologyConstants.ENGINE_MODULE)) {
k.getAsset(urn).ifPresent(Asset::dispose);
}
}
});
pollend marked this conversation as resolved.
Show resolved Hide resolved
assetTypeManager.getAssetType(BlockFamilyDefinition.class).ifPresent(AssetType::disposeAll);
assetTypeManager.getAssetType(Prefab.class).ifPresent(AssetType::disposeAll);
pollend marked this conversation as resolved.
Show resolved Hide resolved


pollend marked this conversation as resolved.
Show resolved Hide resolved
boolean save = networkSystem.getMode().isAuthority();
if (save) {
storageManager.waitForCompletionOfPreviousSaveAndStartSaving();
Expand Down Expand Up @@ -141,6 +161,7 @@ public void dispose(boolean shuttingDown) {
console.dispose();
GameThread.clearWaitingProcesses();


pollend marked this conversation as resolved.
Show resolved Hide resolved
/*
* Clear the binding as otherwise the complete ingame state would be
* referenced.
Expand Down
5 changes: 5 additions & 0 deletions engine/src/main/java/org/terasology/engine/core/modes/StateLoading.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.terasology.engine.core.modes.loadProcesses.InitialiseSystems;
import org.terasology.engine.core.modes.loadProcesses.InitialiseWorld;
import org.terasology.engine.core.modes.loadProcesses.InitialiseWorldGenerator;
import org.terasology.engine.core.modes.loadProcesses.InitialiseRendering;
import org.terasology.engine.core.modes.loadProcesses.JoinServer;
import org.terasology.engine.core.modes.loadProcesses.LoadEntities;
import org.terasology.engine.core.modes.loadProcesses.LoadExtraBlockData;
Expand Down Expand Up @@ -141,6 +142,7 @@ public void init(GameEngine engine) {

private void initClient() {
loadProcesses.add(new JoinServer(context, gameManifest, joinStatus));
loadProcesses.add(new InitialiseRendering(context));
loadProcesses.add(new InitialiseEntitySystem(context));
loadProcesses.add(new RegisterBlocks(context, gameManifest));
loadProcesses.add(new InitialiseGraphics(context));
Expand All @@ -165,6 +167,9 @@ private void initClient() {

private void initHost() {
loadProcesses.add(new RegisterMods(context, gameManifest));
if(netMode.hasLocalClient()) {
loadProcesses.add(new InitialiseRendering(context));
}
loadProcesses.add(new InitialiseEntitySystem(context));
loadProcesses.add(new RegisterBlocks(context, gameManifest));
loadProcesses.add(new InitialiseGraphics(context));
Expand Down
33 changes: 33 additions & 0 deletions engine/src/main/java/org/terasology/engine/core/modes/loadProcesses/InitialiseRendering.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2021 The Terasology Foundation
// SPDX-License-Identifier: Apache-2.0

package org.terasology.engine.core.modes.loadProcesses;

import org.terasology.engine.context.Context;
import org.terasology.engine.core.modes.SingleStepLoadProcess;
import org.terasology.engine.core.module.rendering.RenderingModuleRegistry;

public class InitialiseRendering extends SingleStepLoadProcess {
private final Context context;

public InitialiseRendering(Context context) {
this.context = context;
}


@Override
public String getMessage() {
return "Initialising Rendering System...";
}

@Override
public boolean step() {
context.put(RenderingModuleRegistry.class, new RenderingModuleRegistry());
Copy link
Member

Choose a reason for hiding this comment

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

What makes this better as a LoadProcess instead of a Subsystem?

more explicit ordering of load processes? or we need a different context here than was available for any of the subsystem lifecycle phases?

(and heck it is a lot of lines of code for these two expressions, either way)

Copy link
Member Author

@pollend pollend Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

we need a new rendering system when you restart a world. rendering fails to init correctly when its already been constructed.

Copy link
Member

Choose a reason for hiding this comment

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

For my information: Why do we need a new rendering system when restarting a world?

Copy link
Member Author

@pollend pollend Jul 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

its all module drive there really isn't a reason to keep it around as a singleton. just means we have to manage the lifecycle at a much higher level. introduces extra book keeping that I would like to avoid. anyways one session might not have the same rendering setup so the entire state of the RenderingModuleRegistry would need to be wiped. just easier to dispose of the whole object.

Copy link
Member

Choose a reason for hiding this comment

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

Okay

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.. we do this for quite a few loading things.

Copy link
Member

Choose a reason for hiding this comment

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

Can we please persist this information in a docstring. Probably the class doc here is a good place to capture this.

return true;
}

@Override
public int getExpectedCost() {
return 1;
}
}
70 changes: 0 additions & 70 deletions ...rc/main/java/org/terasology/engine/core/subsystem/rendering/ModuleRenderingSubsystem.java
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions engine/src/main/java/org/terasology/engine/rendering/opengl/GLSLMaterial.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public void bindTextures() {
for (int slot : textureMap.keys()) {
Texture texture = textureMap.get(slot);
if (texture.isDisposed()) {
textureMap.remove(slot);
logger.error("Attempted to bind disposed texture {}", texture);
} else {
shaderManager.bindTexture(slot, texture);
Expand Down
18 changes: 6 additions & 12 deletions engine/src/main/java/org/terasology/engine/world/block/DefaultColorSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,23 @@ public Colorc calcColor(int x, int y, int z) {
COLOR_LUT {
@Override
public Colorc calcColor(int x, int y, int z) {
ColorProvider colorProvider = CoreRegistry.get(ColorProvider.class);
Copy link
Member

Choose a reason for hiding this comment

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

Is doing this lookup in CoreRegistry fast? I think @tolziplohu put this optimization on purpose to reduce the necessary lookups in the registry with each lookup of a color... 🤔

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, but we can't save it as a static variable.

// Return white as default if there aren't any color providers
if (colorProvider == null) {
colorProvider = CoreRegistry.get(ColorProvider.class);
// Return white as default if there aren't any color providers
if (colorProvider == null) {
return Color.white;
}
return Color.white;
}
return colorProvider.colorLut(x, y, z);
}
},
FOLIAGE_LUT {
@Override
public Colorc calcColor(int x, int y, int z) {
ColorProvider colorProvider = CoreRegistry.get(ColorProvider.class);
// Return white as default if there aren't any color providers
if (colorProvider == null) {
colorProvider = CoreRegistry.get(ColorProvider.class);
// Return white as default if there aren't any color providers
if (colorProvider == null) {
return Color.white;
}
return Color.white;
}
return colorProvider.foliageLut(x, y, z);
}
};

private static ColorProvider colorProvider;
}
11 changes: 5 additions & 6 deletions ...e/src/main/java/org/terasology/engine/world/block/loader/BlockFamilyDefinitionFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,24 @@
import com.google.gson.stream.JsonWriter;
import org.joml.Vector3f;
import org.joml.Vector4f;
import org.terasology.engine.world.block.DefaultColorSource;
import org.terasology.gestalt.assets.Asset;
import org.terasology.gestalt.assets.ResourceUrn;
import org.terasology.engine.entitySystem.prefab.Prefab;
import org.terasology.engine.registry.CoreRegistry;
import org.terasology.engine.world.block.shapes.BlockShape;
import org.terasology.engine.world.block.sounds.BlockSounds;
import org.terasology.engine.utilities.gson.CaseInsensitiveEnumTypeAdapterFactory;
import org.terasology.engine.utilities.gson.Vector3fTypeAdapter;
import org.terasology.engine.utilities.gson.Vector4fTypeAdapter;
import org.terasology.engine.world.block.BlockPart;
import org.terasology.engine.world.block.DefaultColorSource;
import org.terasology.engine.world.block.family.BlockFamily;
import org.terasology.engine.world.block.family.BlockFamilyLibrary;
import org.terasology.engine.world.block.family.FreeformFamily;
import org.terasology.engine.world.block.family.HorizontalFamily;
import org.terasology.engine.world.block.family.MultiSection;
import org.terasology.engine.world.block.family.SymmetricFamily;
import org.terasology.engine.world.block.shapes.BlockShape;
import org.terasology.engine.world.block.sounds.BlockSounds;
import org.terasology.engine.world.block.tiles.BlockTile;
import org.terasology.gestalt.assets.Asset;
import org.terasology.gestalt.assets.ResourceUrn;
import org.terasology.gestalt.assets.format.AbstractAssetFileFormat;
import org.terasology.gestalt.assets.format.AssetDataFile;
import org.terasology.gestalt.assets.management.AssetManager;
Expand Down Expand Up @@ -318,7 +318,6 @@ private static class BlockFamilyHandler implements JsonDeserializer<Class<? exte
@Override
public Class<? extends BlockFamily> deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
throws JsonParseException {

Copy link
Member

Choose a reason for hiding this comment

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

All these changes are unrelated to this PR. Please revert them.

BlockFamilyLibrary library = CoreRegistry.get(BlockFamilyLibrary.class);
return library.getBlockFamily(json.getAsString());
}
Expand Down