longturn / freeciv21

Develop your civilization from humble roots to a global empire
GNU General Public License v3.0
220 stars 42 forks source link

Clean up hud_text #1251

Open lmoureaux opened 2 years ago

lmoureaux commented 2 years ago

Is your feature request related to a problem? Please describe.

hud_text's coding style is... nonstandard. It looks like someone frantically tried to get the fade-in and fade-out effects working and forgot to clean up afterwards. This issue is about bringing it to a higher Qt standard using its Animation Framework. Detailed steps follow.

Describe the solution you'd like

The main parameter that changes during animation is the opacity of the widget. So first make it a class member variable and move the updates to timerEvent(). Next, we would like to make the opacity available to the animation framework. We'll want to use QPropertyAnimation, so turn the m_opacity private member into a fully-fledged opacity property with a getter and a setter. The setter should call QWidget::update() to request a repaint.

Currently, the opacity animation proceeds in 3 steps:

We can assume that the intention for the first half-second was to have the opacity go from 0 to 1, and that there is an error in the implementation.

In Qt, sequences like the above are represented using a QSequentialAnimationGroup. Assuming we are in the hud_text constructor:

auto sequence = new QSequentialAnimationGroup(this);

auto fade = new QPropertyAnimation(this, "opacity");
fade->setStartValue(0);
fade->setEndValue(1);
fade->setDuration(500);
sequence->addAnimation(fade);

sequence->addPause(3500);

fade = new QPropertyAnimation(this, "opacity");
fade->setStartValue(1);
fade->setEndValue(0);
fade->setDuration(1500);
sequence->addAnimation(fade);

sequence->start();

The last step is to delete the widget once we reach the end of the animation. For this, we can connect the finished() signal of sequence to the deleteLater() function of hud_text. It's not super clean, but will do the job.

This replaces of all of the timer stuff in hud_widget. There is some more cleaning one could do:

Describe alternatives you've considered

Doing it myself would've been faster :smile:

Additional context

1246

jwrober commented 2 years ago

It also does not properly support the QMessageBox class as discovered while working on #1386. It does a bunch of custom crazy stuff that doesn't work in all cases.