MovingBlocks / Terasology

Terasology - open source voxel world
http://terasology.org
Apache License 2.0
3.69k stars 1.34k forks source link

Add informative popup if trying to start a game without modules #4034

Closed Cervator closed 4 years ago

Cervator commented 4 years ago

What you were trying to do

Test that the game handles the complete absence of modules well

What actually happened

Mystery lack of action trying to click the "Play Now" button with no gameplay template selected (since there are no modules). Would expect to get an error popup explaining that you can't start playing with no modules!

How to reproduce

Tip: Look in the NewGameScreen class for the associated UI logic.

ayrustogaru commented 4 years ago

@Cervator I'm a novice. Can I have a go at this?

pollend commented 4 years ago

have a go at it, if you need some extra guidance you can join the discord.

Cervator commented 4 years ago

Hi @ayrustogaru - yep, absolutely, as noted already by pollend :-)

The biggest obstacle tends to be getting stuck yet not asking questions, so please do hop on chat if you get stuck on anything

You may want to look at https://github.com/Terasology/TutorialNui/wiki but we just majorly worked on a series of fundamental systems so I'm not sure if that tutorial is still up to date. That'd be another simple way to help: try out that tutorial and see if it works and makes sense!

ayrustogaru commented 4 years ago

@pollend @Cervator thanks, I'm taking a look at it now. Will surely drop a question on discord if I get stuck.

ayrustogaru commented 4 years ago

@Cervator or @pollend. I have singled out the error in the NewGameScreen class. The problem is that there was an error popup created, but it wasn't pushed to the screen. So, I'll just pushScreen() it.

GameManifest gameManifest = GameManifestProvider.createGameManifest(universeWrapper, moduleManager, config);
if (gameManifest != null) {
    gameEngine.changeState(new StateLoading(gameManifest, (isLoadingAsServer()) ? NetworkMode.DEDICATED_SERVER : NetworkMode.NONE));
} else {
    MessagePopup errorPopup = getManager().createScreen(MessagePopup.ASSET_URI, MessagePopup.class);
    errorPopup.setMessage("Error", "Can't create new game!");
}

Now, my doubt is should I leave the error message as it is or make any changes to it (to make it informative) ?

Cervator commented 4 years ago

So "Can't create new game!" is indeed not very informative. Is there something we can check to base a better error message on?

In the case of this issue the problem is that there are no modules, but if that error popup can also trigger from other causes it might not be good to just change to a message specifically about no modules. But if we can check something (moduleManager maybe?) and find out for sure that there are no modules, then in that case at least we can use a specific error. In fact, probably that could be checked right before even trying to create the gameManifest ?

ayrustogaru commented 4 years ago

I think I came up with a workaround. There was a UIDropDownScrollable<Module> object called gameplay whose options were set to the different gameplay modules. So, I checked if there were any options set in it to decide whether there are any modules present.

if (gameplay.getOptions().isEmpty()) {
       MessagePopup errorPopup = getManager().pushScreen(MessagePopup.ASSET_URI, MessagePopup.class);
       errorPopup.setMessage("Error", "Can't create new game without modules");
}

That is the error message I'll be printing. Do I put this in the logger too? Now, Is this train of thought ok, or does this have any limitations I am missing?

Cervator commented 4 years ago

@ayrustogaru looks good to me! PR away :-)

And in general feel free to PR early and often. It can be easier to discuss the technical details in a PR with attached code changes, and you can always mark it as a draft until happy with it. No obligation or anything, just a tip!

You could put a logger in there as well, yeah, just for completeness

ayrustogaru commented 4 years ago

Sorry about that, I'll try to PR early from the next time. Thanks for the tip😁.