-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
refactor!: remove multi-world support #5116
Conversation
@skaldarnar refactoring steps so far remove the option to add more than one world and removes respective code, in particular the former world lists are now only single WorldSetupWrappers... |
This removes UI widgest referencing the following translation keys: - `engine:menu#game-worlds` - `engine:menu#add` - `engine:menu#config`
...e/src/main/java/org/terasology/engine/rendering/nui/layers/mainMenu/UniverseSetupScreen.java
Outdated
Show resolved
Hide resolved
@@ -242,4 +180,4 @@ | |||
} | |||
] | |||
} | |||
} | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline
@@ -291,6 +291,7 @@ private void initAssets() { | |||
* in the drop-down. | |||
* @return A list of world names encoded as a String | |||
*/ | |||
@Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we still need this?
@@ -265,4 +263,4 @@ | |||
} | |||
] | |||
} | |||
} | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skaldarnar your changes look fine to me.
can you also review the changes I did so we have a two-way approval?
@@ -0,0 +1,6 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does this file come from? should it be committed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to https://intellij-support.jetbrains.com/hc/en-us/articles/206544839-How-to-manage-projects-under-Version-Control-Systems it should be checked in 🤷
@@ -70,19 +65,8 @@ public void initialise() { | |||
|
|||
SimpleUri uri; | |||
WorldInfo worldInfo; | |||
//TODO: if we don't do that here, where do we do it? or does the world not show up in the game manifest? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done as part of GameManifestProvider#createGameManifest
which is called on the other code paths, I think.
Line 96 in 1d831ea
gameManifest.addWorld(worldInfo); |
We can remove this TODO comment and the line below.
Contains
Remove code and abstractions related to mutli-word support during the "Advanced" game creation flow.
Context
The multi-world feature was added as a GSOC project back in 2018 by @TheFlash98. For more information, checkout the original forum post GSOC 2018 - Multiple Worlds or the project page.
While the world generation flow now supports to "create" multiple worlds and configure them separately, these multiple worlds were never experianceable in-game.
How to test
Create a new game via the "Advanced" game setup flow.
Outstanding before merging
WorldSetupWrapper
s in screen classesFollow-ups