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

zenseii · 2025-01-29T12:53:03Z

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

Fix #8720

WIP. Need correct placing of button and design of new dialog

github-actions

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

src/fheroes2/editor/editor_interface.cpp

Mr-Bajs · 2025-01-30T16:26:51Z

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.

Districh-ru

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

Districh-ru · 2025-01-30T16:48:17Z

src/fheroes2/editor/editor_interface.cpp

+        fheroes2::Sprite mainMenuReleased;
+        fheroes2::Sprite mainMenuPressed;
+        fheroes2::getTextAdaptedButton( mainMenuReleased, mainMenuPressed, gettext_noop( "MAIN\nMENU" ),
+                                        isEvilInterface ? ICN::EMPTY_EVIL_BUTTON : ICN::EMPTY_GOOD_BUTTON, isEvilInterface ? ICN::STONEBAK_EVIL : ICN::STONEBAK );
+        fheroes2::Sprite playMapReleased;
+        fheroes2::Sprite playMapPressed;
+        fheroes2::getTextAdaptedButton( playMapReleased, playMapPressed, gettext_noop( "PLAY\nMAP" ), isEvilInterface ? ICN::EMPTY_EVIL_BUTTON : ICN::EMPTY_GOOD_BUTTON,
+                                        isEvilInterface ? ICN::STONEBAK_EVIL : ICN::STONEBAK );


Should we generate these button's images in agg_file.cpp like it is done for the other buttons?

We want to avoid polluting agg_file.cpp file as it became too big to maintain. Having a function to do the work without increasing the number of ICN entries is a better solution.

@zenseii , I believe fheroes2::getTextAdaptedButton() function should be renamed into fheroes2::getTextAdaptedButtonSprites() function as it doesn't generate a button but only creates Sprites.

Going forward, we can create fheroes2::getTextAdaptedButton() function which will return fheroes2::ButtonSprite object so we can use it here. What do you think?

So then it should be better to remove all other buttons from agg_image.cpp used in this dialog and generate them here to have the same code and to avoid questions like "Why this button image is made this way and that button - the other way?" ;)

@ihhub and @Districh-ru. I will pause this PR and make a new one that makes these changes so that we can tidy up the button code and avoid confusion moving forward.

Hi @Districh-ru , not all buttons from agg_file.cpp file can be removed easily. Some of them have logic for button generation, others support original resources.

Districh-ru · 2025-01-30T17:15:18Z

src/fheroes2/editor/editor_interface.cpp

+                    }
+                }
+                else {
+                    fheroes2::showStandardTextMessage( _( "Invalid Map" ), _( "This map is not valid. Try to save the latest changes." ), Dialog::OK );


Should we say about the need of 1 or more players on the map?
In example: "This map is not valid. The map should have at least one human player and be saved on your storage."

Districh-ru · 2025-01-30T17:16:40Z

src/fheroes2/editor/editor_interface.cpp

+            if ( le.MouseClickLeft( buttonPlayMap.area() ) ) {
+                // The map either hasn't been saved to a file or there aren't any human-playable colors.
+                if ( !conf.getCurrentMapInfo().name.empty() && conf.getCurrentMapInfo().colorsAvailableForHumans > 0 ) {
+                    if ( fheroes2::showStandardTextMessage( _( "Play Map" ), _( "Do you wish to leave the editor and play the current map?" ), Dialog::YES | Dialog::NO )


Should we mention that all unsaved changes will be lost or allow to open the Save Map dialog?

Districh-ru · 2025-01-30T17:21:32Z

src/fheroes2/editor/editor_interface.cpp

+            else if ( le.isMouseRightButtonPressedInArea( buttonMainMenu.area() ) ) {
+                fheroes2::showStandardTextMessage( _( "Main Menu" ), _( "Return to the game's Main Menu." ), Dialog::ZERO );
+            }


It would be nice if we add such dialog for the PLAY MAP button :)

zenseii · 2025-01-30T18:21:06Z

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 · 2025-01-30T18:35:38Z

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.

ihhub · 2025-01-31T04:12:42Z

src/fheroes2/editor/editor_interface.cpp

+        fheroes2::Sprite mainMenuReleased;
+        fheroes2::Sprite mainMenuPressed;
+        fheroes2::getTextAdaptedButton( mainMenuReleased, mainMenuPressed, gettext_noop( "MAIN\nMENU" ),
+                                        isEvilInterface ? ICN::EMPTY_EVIL_BUTTON : ICN::EMPTY_GOOD_BUTTON, isEvilInterface ? ICN::STONEBAK_EVIL : ICN::STONEBAK );
+        fheroes2::Sprite playMapReleased;
+        fheroes2::Sprite playMapPressed;
+        fheroes2::getTextAdaptedButton( playMapReleased, playMapPressed, gettext_noop( "PLAY\nMAP" ), isEvilInterface ? ICN::EMPTY_EVIL_BUTTON : ICN::EMPTY_GOOD_BUTTON,
+                                        isEvilInterface ? ICN::STONEBAK_EVIL : ICN::STONEBAK );


We want to avoid polluting agg_file.cpp file as it became too big to maintain. Having a function to do the work without increasing the number of ICN entries is a better solution.

@zenseii , I believe fheroes2::getTextAdaptedButton() function should be renamed into fheroes2::getTextAdaptedButtonSprites() function as it doesn't generate a button but only creates Sprites.

Going forward, we can create fheroes2::getTextAdaptedButton() function which will return fheroes2::ButtonSprite object so we can use it here. What do you think?

ihhub · 2025-01-31T04:14:26Z

src/fheroes2/editor/editor_interface.cpp

+            if ( le.MouseClickLeft( buttonPlayMap.area() ) ) {
+                // The map either hasn't been saved to a file or there aren't any human-playable colors.
+                if ( !conf.getCurrentMapInfo().name.empty() && conf.getCurrentMapInfo().colorsAvailableForHumans > 0 ) {
+                    if ( fheroes2::showStandardTextMessage( _( "Play Map" ), _( "Do you wish to leave the editor and play the current map?" ), Dialog::YES | Dialog::NO )


Should Editor start from a capital letter?

zenseii · 2025-01-31T11:29:26Z

@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 · 2025-01-31T13:31:10Z

@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 · 2025-01-31T13:35:45Z

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).

zenseii added 2 commits January 29, 2025 13:45

Add Main Menu button to editor's file options dialog

3859f65

WIP. Need correct placing of button and design of new dialog

Code style and right click info

d2d8b26

zenseii added improvement New feature, request or improvement ui UI/GUI related stuff editor Map editor related stuff labels Jan 29, 2025

zenseii added this to the 1.1.6 milestone Jan 29, 2025

github-actions bot assigned zenseii Jan 29, 2025

github-actions bot reviewed Jan 29, 2025

View reviewed changes

src/fheroes2/editor/editor_interface.cpp Outdated Show resolved Hide resolved

src/fheroes2/editor/editor_interface.cpp Outdated Show resolved Hide resolved

zenseii added 2 commits January 29, 2025 13:56

Fix equal conditional branches

69ba7c2

IWYU, change confirm dialog header

ed5d686

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 added 5 commits January 30, 2025 12:20

Make the buttons aligned horizontally

c93771d

Add play map button

07970f6

early returns and tidying

b403bf1

Add Play Map button, restorer for save dialog

e81d8ff

clang tidy

0e9e4a7

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 marked this pull request as ready for review January 30, 2025 15:05

zenseii requested review from ihhub, Districh-ru and Branikolog January 30, 2025 15:58

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

Districh-ru reviewed Jan 30, 2025

View reviewed changes

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

ihhub reviewed Jan 31, 2025

View reviewed changes

Fix fade-in when PLAY MAP is pressed

8c86d1e

Update copyright

1bc62ce

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

zenseii commented Jan 29, 2025 •

edited

Loading

github-actions bot left a comment

Mr-Bajs commented Jan 30, 2025 •

edited

Loading

Districh-ru left a comment

Districh-ru Jan 30, 2025

ihhub Jan 31, 2025

Districh-ru Jan 31, 2025

zenseii Jan 31, 2025

ihhub Jan 31, 2025

Districh-ru Jan 30, 2025

Districh-ru Jan 30, 2025

Districh-ru Jan 30, 2025

zenseii commented Jan 30, 2025

Mr-Bajs commented Jan 30, 2025

ihhub Jan 31, 2025

ihhub Jan 31, 2025

zenseii commented Jan 31, 2025

Districh-ru commented Jan 31, 2025

Districh-ru commented Jan 31, 2025

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

Are you sure you want to change the base?

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

Conversation

zenseii commented Jan 29, 2025 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

Mr-Bajs commented Jan 30, 2025 • edited Loading

Districh-ru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zenseii commented Jan 30, 2025

Mr-Bajs commented Jan 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zenseii commented Jan 31, 2025

Districh-ru commented Jan 31, 2025

Districh-ru commented Jan 31, 2025

zenseii commented Jan 29, 2025 •

edited

Loading

Mr-Bajs commented Jan 30, 2025 •

edited

Loading