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

Modernize cursor update for the game area #8012

Closed
wants to merge 13 commits into from

Conversation

Districh-ru
Copy link
Collaborator

@Districh-ru Districh-ru commented Nov 2, 2023

Based on #7808.

This PR does the next changes:

  • fixes Map Editor, cursor issues after info window appears #8026;
  • pass gameArea to the QuickInfo() functions;
  • restore cursor immediately after the Quick Info dialog is closed if no cursor change is predicted;
  • schedule immediate cursor update after Quick Info dialog is closed;
  • set POINTER cursor when map dragging is started (it is made to show the player that no action will be performed after the mouse button is released, discussible, can be reverted);
  • add forced software cursor render on the screen after cursor icon is updated;
  • remove the forced cursor update setting on every CURRENT_HERO_DELAY pass: this part needs testing, the assertions are temporary put instead;
  • schedule the cursor update after Puzzle or View World dialog is closed;
  • make View World use reference to the gameArea of given interface (AdventureMab/EditorInterface);
  • do not make gameArea copies for the View World, use references to the original gameArea.

@Districh-ru Districh-ru added improvement New feature, request or improvement ui UI/GUI related stuff labels Nov 2, 2023
@Districh-ru Districh-ru added this to the 1.0.10 milestone Nov 2, 2023
@Districh-ru Districh-ru self-assigned this Nov 2, 2023
@Districh-ru Districh-ru marked this pull request as ready for review November 3, 2023 18:35
@Districh-ru Districh-ru added the bug Something doesn't work label Nov 6, 2023
Copy link
Collaborator

@Branikolog Branikolog left a comment

Choose a reason for hiding this comment

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

Everything works fine.
However, this PR does not fix #8026.

@ihhub
Copy link
Owner

ihhub commented Nov 9, 2023

I am moving this change to the next release as we are very close to the current release.

@ihhub ihhub modified the milestones: 1.0.10, 1.1.0 Nov 9, 2023
Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @Districh-ru , I put several comments in this pull request. Would you mind to take a look at them?

src/fheroes2/game/game_startgame.cpp Outdated Show resolved Hide resolved
src/fheroes2/game/game_startgame.cpp Outdated Show resolved Hide resolved
src/fheroes2/gui/cursor.cpp Outdated Show resolved Hide resolved
src/fheroes2/dialog/dialog_quickinfo.cpp Outdated Show resolved Hide resolved
src/fheroes2/gui/cursor.cpp Outdated Show resolved Hide resolved
src/fheroes2/gui/interface_gamearea.cpp Outdated Show resolved Hide resolved
Comment on lines 204 to 206
adventureMapInterface.getGameArea().SetUpdateCursor();

adventureMapInterface.setRedraw( Interface::REDRAW_RADAR_CURSOR );
Copy link
Owner

Choose a reason for hiding this comment

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

I can see that we set to update cursor if Interface::REDRAW_RADAR_CURSOR flag is set. Is it valid for all situations? If yes should we add an internal logic while redrawing adventure map to update the cursor?

Copy link
Collaborator Author

@Districh-ru Districh-ru Nov 19, 2023

Choose a reason for hiding this comment

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

Hi, @ihhub, you are right - REDRAW_RADAR_CURSOR is not needed here because we restore radar by restorer. I made the same restorer for the second puzzle function. It was called here to restore radar because its area is used to render EXIT button.

@Districh-ru Districh-ru marked this pull request as draft November 18, 2023 03:31
@Districh-ru Districh-ru marked this pull request as ready for review November 19, 2023 13:32
@ihhub ihhub modified the milestones: 1.1.0, 1.0.12 Jan 13, 2024
@ihhub
Copy link
Owner

ihhub commented Jan 13, 2024

Hi @Districh-ru , do we need to change specific logic in this pull request since we separated Adventure Map and Editor classes and do not share quick info dialog code for the Editor?

@Districh-ru Districh-ru marked this pull request as draft January 14, 2024 07:32
@Districh-ru
Copy link
Collaborator Author

Hi @Districh-ru , do we need to change specific logic in this pull request since we separated Adventure Map and Editor classes and do not share quick info dialog code for the Editor?

Hi @ihhub, yes this PR needs some improvement for the new editor quick view. I'll update it on the next week.

@Districh-ru
Copy link
Collaborator Author

Some parts of this PR are redone in separate PRs.
QuickInfo() was reworked recently and needs many conflicts to resolve.
Besides the approach in this PR is not perfect and involves many duplications and the need to transfer the gameArea instance to the QuickInfo() functions.
I'm closing this PR and will open a new one to fix QuickInfo() cursor update after I test another approach.

@Districh-ru Districh-ru deleted the gamearea-cursor-update branch January 21, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work improvement New feature, request or improvement ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map Editor, cursor issues after info window appears
3 participants