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

Add return to main menu and play map buttons to the editor #9512

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

zenseii
Copy link
Collaborator

@zenseii zenseii commented Jan 29, 2025

I had to add an extra button to keep symmetry of the dialog. The button can easily be changed to "test map" or similar.

image

Fix #8720

WIP. Need correct placing of button and design of new dialog
@zenseii zenseii added improvement New feature, request or improvement ui UI/GUI related stuff editor Map editor related stuff labels Jan 29, 2025
@zenseii zenseii added this to the 1.1.6 milestone Jan 29, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/fheroes2/editor/editor_interface.cpp Outdated Show resolved Hide resolved
src/fheroes2/editor/editor_interface.cpp Outdated Show resolved Hide resolved
@zenseii zenseii changed the title Add button to return to main menu from editor Add button to return to main menu from the editor Jan 29, 2025
@zenseii zenseii changed the title Add button to return to main menu from the editor Add button to return to main menu and play map buttons to the editor Jan 30, 2025
@zenseii zenseii marked this pull request as ready for review January 30, 2025 15:05
@Mr-Bajs
Copy link
Contributor

Mr-Bajs commented Jan 30, 2025

Looks nice. But would it look better if Load Map and Save Map where next to each other in the same row or column? Not that it really matters.

@zenseii zenseii changed the title Add button to return to main menu and play map buttons to the editor Add buttons to return to main menu and play map to the editor Jan 30, 2025
Copy link
Collaborator

@Districh-ru Districh-ru left a comment

Choose a reason for hiding this comment

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

Hi, @zenseii, I left several suggestions. Could you please take a look when you have time?

src/fheroes2/editor/editor_interface.cpp Show resolved Hide resolved
src/fheroes2/editor/editor_interface.cpp Show resolved Hide resolved
src/fheroes2/editor/editor_interface.cpp Show resolved Hide resolved
src/fheroes2/editor/editor_interface.cpp Show resolved Hide resolved
@zenseii
Copy link
Collaborator Author

zenseii commented Jan 30, 2025

Looks nice. But would it look better if Load Map and Save Map where next to each other in the same row or column? Not that it really matters.

Hi, @Mr-Bajs. I wanted to mimic as much as possible how the original buttons were placed but I'm really impartial to it. I guess Main Menu should be where it is and quit should be lower right, the rest is the same for me.

@Mr-Bajs
Copy link
Contributor

Mr-Bajs commented Jan 30, 2025

Looks nice. But would it look better if Load Map and Save Map where next to each other in the same row or column? Not that it really matters.

Hi, @Mr-Bajs. I wanted to mimic as much as possible how the original buttons were placed but I'm really impartial to it. I guess Main Menu should be where it is and quit should be lower right, the rest is the same for me.

Swapping places save map and play map might look better. But since also the same menu ingame would also benefit with a Main Menu button. You layout keeps the menus more similar to each other maybe thats better, atleast until or if the in game menu gets an update with a main menu button.

@zenseii zenseii changed the title Add buttons to return to main menu and play map to the editor Add return to main menu and play map buttons to the editor Jan 30, 2025
@zenseii
Copy link
Collaborator Author

zenseii commented Jan 31, 2025

@Districh-ru. I noticed that when the Play Map button is used and you arrive at the scenario info dialog, if you then press cancel the screen will flash as if a fadeout was triggered. Are fade out's somehow cued? I'm not entirely sure where to look to solve this.

@Districh-ru
Copy link
Collaborator

@Districh-ru. I noticed that when the Play Map button is used and you arrive at the scenario info dialog, if you then press cancel the screen will flash as if a fadeout was triggered. Are fade out's somehow cued? I'm not entirely sure where to look to solve this.

@zenseii, I've fixed this issue - added a check when rendering the Select Scenario dialog that also resets the need of fade effect.
Previously it was not needed because there was no way to open this dialog bypassing the main menu. :)

@Districh-ru
Copy link
Collaborator

Looks nice. But would it look better if Load Map and Save Map where next to each other in the same row or column? Not that it really matters.

Hi, @Mr-Bajs. I wanted to mimic as much as possible how the original buttons were placed but I'm really impartial to it. I guess Main Menu should be where it is and quit should be lower right, the rest is the same for me.

I'd also prefer to swap PLAY MAP and SAVE MAP so there the upper row will be for the map manipulations (new/load/save) and the lower row for the quit from Editor (to new game/main menu/OS).

@ihhub
Copy link
Owner

ihhub commented Feb 2, 2025

I am marking this pull request as high priority as it is a must for the upcoming release.

@ihhub ihhub added the high priority Very critical change needed immediately label Feb 2, 2025
@Branikolog
Copy link
Collaborator

Hi, @zenseii !

New menu looks great! 👍 👍 👍

It would be nice to add an info hint on right click for the new "Play Map" button too.

I'm also slightly confused by the term "Invalid map" in this window:
image

It's a nice touch to warn a player, that he cannot play unsaved map, but in my personal opinion we shouldn't call this map as "invalid" as it misleads to the fact, that something is wrong with the map itself. Maybe it's better to point that user cannot play an unsaved map.

I also prefer Save and Load buttons to be placed on current different levels, as players can accidentally press wrong button and spoil his map. 👍

I've also noticed, that gaps between buttons are not same:
image

@zenseii
Copy link
Collaborator Author

zenseii commented Feb 2, 2025

Hi, @Branikolog, @Districh-ru and @Mr-Bajs. The placement of the buttons will need to be figured out. As I said, it's not very important for me how it is, as long as we arrive at a conclusion. My reasoning was only that the New map, Save map and load map are now placed similarly to were they were before. I suppose we could swap places with Main menu and Quit to make it even more similar.

@Branikolog, the dialog was added very quickly and I'm open to suggestions. The reason it not only says that the player has to save the map is because the message also pops up if the map can't be played, like if there are zero heroes placed that can be played by a human player. This is the same criteria we have for showing maps in the map select dialog, but again, I'm open to suggestions for a dialog text that fits this better. We could say "Unplayable map" because any map that has changes that aren't saved is unplayable currently, as @Districh-ru we can make it so that a save map can be shown in this case.

@Branikolog
Copy link
Collaborator

@zenseii

Current button placement is fine for me.

We could say "Unplayable map" because any map that has changes that aren't saved is unplayable currently,

In my opinion there should be two separate cases, when the saved map cannot be played, because of incorrect game conditions (Invalid is ok then) and when the changes just haven't been saved yet.
For the last case there could be a message referring to unsaved process of the mapmaking, something like:
"You cannot play the map, until you save all changes. Save the current progress and try again."

@zenseii zenseii mentioned this pull request Feb 3, 2025
@zenseii
Copy link
Collaborator Author

zenseii commented Feb 6, 2025

I've made the necessary changes to this PR and will push them once the PR #9525 has been completed since it requires almost all of the changes there.

@LeHerosInconnu
Copy link

Hello everyone,

The button layout could be done this way:
Image from the screenshot of the original post, for explanation only.
So that the buttons are positioned "in pairs" (vertically from left to right).

  • "NEW MAP" "PLAY MAP"
  • "LOAD MAP" "SAVE MAP"
  • "MAIN MENU" "QUIT".

Button layout 01 001

What do you think of "LAUNCH MAP" to replace "PLAY MAP"?

Related issue: #2109 (for button layout in pairs).

@zenseii
Copy link
Collaborator Author

zenseii commented Feb 7, 2025

Hi, @LeHerosInconnu.

I tested using LAUNCH MAP and it doesn't fit and it will be quite a while until we can adjust the size of all of these buttons and there's a point to having them all the same size. There's also the point that all the other buttons either have 3- or 4-letter words (SAVE,LOAD,MAIN,MENU,QUIT, MAP,NEW), and "PLAY" fits in with that.

image

As for the button placements, as I said before I don't have any preferences. @Branikolog and @Districh-ru , you've already decided what you prefer. That leaves the final decision up to @ihhub.

EDIT: I suppose we haven't considered "TRY MAP", though its meaning is similar to test map:
image

image

START MAP actually fits and though it breaks the 3/4-letter rule, if we implement RESTART MAP in the adventure map file dialog, then these two could be considered counterparts and probably have the same placings:
image

@LeHerosInconnu
Copy link

Hello @zenseii,

Yes, "START MAP" would be more appropriate.

@Branikolog
Copy link
Collaborator

I'm Ok with both "Play" or "Start".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editor Map editor related stuff high priority Very critical change needed immediately improvement New feature, request or improvement ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: Exit option should redirect back to game main menu
6 participants