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

Fix Battlefield animations freeze while waiting for Good Luck sound to end #7061

Merged
merged 4 commits into from
May 7, 2023

Conversation

Districh-ru
Copy link
Collaborator

This PR fixes the freeze of Battlefield animations after finishing rainbow animation and until Good Lock sound is over.
This PR also removes waiting for Good Luck sound if sound volume is set to 0.

…end, don't wait for Good Luck sound when sounds are turned off
@Districh-ru Districh-ru added improvement New feature, request or improvement ui UI/GUI related stuff labels Apr 25, 2023
@Districh-ru Districh-ru added this to the 1.0.4 milestone Apr 25, 2023
@Districh-ru Districh-ru self-assigned this Apr 25, 2023
@ihhub
Copy link
Owner

ihhub commented Apr 25, 2023

Hi @Districh-ru , could you please explain why not playing sound is applicable only for Luck? We have a lot of other sounds during battle which have the same issue.

@Districh-ru
Copy link
Collaborator Author

Districh-ru commented Apr 25, 2023

Hi @Districh-ru , could you please explain why not playing sound is applicable only for Luck? We have a lot of other sounds during battle which have the same issue.

Hi, @ihhub, here if we play the Luck sound we wait for it to over even of the rainbow animation is finished. I played a little with disabled sounds and at the 10th speed wile making some tests and had to watch at quickly finished rainbow, waiting for the silenced Luck sound to over.
The idea was taken from pre-battle sound. :)

Or you mean to use the same approach (not wait for silecend sound to end) for other battle effects where Mixer::isPlaying( -1 ) is analyzed? (Bad Luck, Teleport, Blood Lust, Stone Spells)

@ihhub ihhub requested a review from Branikolog May 4, 2023 15:45
@Branikolog
Copy link
Collaborator

Hi, @Districh-ru
I've tested this PR.
Correct me, if I'm wrong. This PR adds idle animations after rainbow animation is finished, but still stays above the creature, till sound is over, right? During my midnight tests I've created a discussion, thinking I was testing master branch which has animations stopped. :D
I personally love, how animations are freezed for a while, as I described in #7107. So I'm not sure in this PR implementation of idle animations.
Speaking about no sound case - I think it's a valid improvement. But I actually can't see much difference in final rainbow stage lasting on 10 speed with sound off. On the lower speeds it is more noticeable. Did you set some minimal timings so there's a certain pause for the highest speeds?

@Districh-ru
Copy link
Collaborator Author

Districh-ru commented May 5, 2023

Hi, @Branikolog!
Thanks for the review.

Correct me, if I'm wrong. This PR adds idle animations after rainbow animation is finished, but still stays above the creature, till sound is over, right?

Yes, you are right. Currently in master build while the rainbow is "going" from the edge of the screen to the creature all animations, including IDLE, are going and when the rainbow animation is finished the whole screen (hero's flag, Genies, Zombie's flies, ...) freezes.
This PR fixes this freeze.
I'll try to make all creatures except Genies, Ghosts and Zombies to stop their idle animation. :)

Did you set some minimal timings so there's a certain pause for the highest speeds?

Currently we have only the time of Luck sound and if the Rainbow animation finishes earlier we freeze the screen and wait until the sound is over.
As the rainbow animation lasts for some time (also on the highest speed) so I don't think we need some extra time to make player notice that the Good Luck was applied.
Current master build:

fheroes2.2023-05-05.17-22-37-467.mp4

Added:
This PR (after the proposed changes):

fheroes2.2023-05-05.21-54-42-508.1.mp4

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.

Looks well. Works faster for no sound battle. No more freezes at all during battle.

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 left one question. Could you please take a look when you have time?

src/fheroes2/battle/battle_interface.cpp Outdated Show resolved Hide resolved
@ihhub ihhub merged commit 9660280 into ihhub:master May 7, 2023
@ihhub
Copy link
Owner

ihhub commented May 7, 2023

@Districh-ru , thank you very much for this fix!

@Districh-ru Districh-ru deleted the rainbow_animation_fix branch May 7, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature, request or improvement ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants