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

Updated tutorial and input map bug #630

Closed
wants to merge 3 commits into from
Closed

Updated tutorial and input map bug #630

wants to merge 3 commits into from

Conversation

Sceptres
Copy link

@Sceptres Sceptres commented Oct 16, 2021

Description

  1. Updated the tutorial to include a note on how the view zooms out while accelerating as in issue Improve in-game tutorial #628.
  2. Also fixed issue Trivial: if ship is turning to meet mouse cursor and you open the map, you keep spinning #614 and only allowed controls to work if the user is focused on the game screen.

Testing

  1. Ran tutorial multiple times to ensure text is within the box range
    and that the tutorial moves on as expected to the next part
  2. Ran steps 2-3 of issue Trivial: if ship is turning to meet mouse cursor and you open the map, you keep spinning #614 and opened the map, as well as accelerating in the opposite direction of the map button then opening the map.

Outstanding

  1. Add the changing ship step into the tutorial
  2. Add mercenaries step into the tutorial

Pre Pull Request Checklist:

  • Code has been scanned with SonarLint
  • There are no errors present in the project
  • Code has been formatted and indented
  • Methods have appropriate Javadoc (How to write Javadoc)

@Sceptres Sceptres changed the title Updated tutorial to include note on view zoom out and speed Updated tutorial and input map bug Oct 17, 2021
Copy link
Contributor

@BenjaminAmos BenjaminAmos left a comment

Choose a reason for hiding this comment

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

Thank you for fixing these bugs! I've left a couple of suggestions about how to improve this. Feel free to discuss anything I've mentioned if you feel differently about it.

Comment on lines +189 to +195
addStep("Note that the view zooms\nout at high speeds!", upCtrl);
} else if (mobile) {
addStep("Move forward.\nThere's no stop!", nuiUpCtrl);
addStep("Note that the view zooms\nout at high speeds!", nuiUpCtrl);
} else {
addStep("Move forward (" + gameOptions.getKeyUpName() + " key).\nThere's no stop!", nuiUpCtrl);
addStep("Note that the view zooms\nout at high speeds!", nuiUpCtrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

I was a bit confused at first about how to advance the game past this message when I tested the tutorial. How about changing the next message trigger key to shootCtrl? Also, you might want to give the player a hint about which button to press.

Later-on in the tutorial, there's a series of informative steps (hints) that work in the way I was thinking this one should. They are just created like this.

addStep("Here's a couple of hints...\n" + shootKey2, shootCtrl);

The text exceeding the bounds of its container has been an issue for a long time. We're moving over to a new UI framework (NUI) gradually, so hopefully when this screen gets updated for it then that issue will go away. Until then, you'll just have to test that the screens fit at different resolutions manually.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not entirely sure about the wording of the hint but writing style is a very personal choice, so I won't make any firm suggestions here. Try to make it fit in with the more imperative style that the rest of the tutorial uses if you can. Maybe this is fine as-is though.

@Sceptres Sceptres closed this Nov 1, 2021
@Cervator
Copy link
Member

Cervator commented Nov 1, 2021

Hey @Sceptres - we're still eager to get these improvements merged when ready :-) Are you planning to just re-open with the remaining tweaks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants