helge17 / tuxguitar

Improve TuxGuitar and provide builds
Other
435 stars 35 forks source link

Rationalize player icons #379

Closed guiv42 closed 4 months ago

guiv42 commented 4 months ago

Player icons are defined by pairs, with suffixes _1 and _2 :

Example: for Oxygen skin, transport_first_1 and transport_first_2

However, for most skins the 2 icons of a pair are exactly identical. Exceptions:

Looking in the code (at several places), icon _1 or _2 is selected considering condition MidiPlayer.isPaused(). However this does not correspond to any logical state of the midi player. Whenever user clicks pause or stop, the player stops. See this discussion

Some cleanup seems needed here.

helge17 commented 4 months ago

I would say that the dark controls should mean „Player is turned off“ (replay stopped) and the bright controls mean „Player is turned on“ (playing or paused).

The player in 1.5.6 and in the new version behaves slightly differently, see green/red markings.

image

So 1.5.6 distinguishes between the "Pause" status and the "Stop" status, but makes quite unexpected cursor movements.

The new version places the cursor more logically, but does not handle the "Pause" status correctly.

guiv42 commented 4 months ago

Thanks for this analysis, this is more clear.

So 1.5.6 distinguishes between the "Pause" status and the "Stop" status, but makes quite unexpected cursor movements.

I'd say this is questionable. Start playing, then pause somewhere. Player is considered "paused". But is it really? Start editing the song, adding or removing measures. Can we still considered the player is "paused"? Since, when hitting "play" the starting point of the player cannot even be compared with the place where "pause" was hit?

The new version places the cursor more logically, but does not handle the "Pause" status correctly.

That's right. In fact it just means the concept of "paused" status is not well defined.

So, how can we move forward? My proposal: just keep one single version for each icon

helge17 commented 4 months ago

My proposal: just keep one single version for each icon

You're right. So I will rename all transport_1.png icons to transport.png and remove the transport*_2.png icons, ok?

guiv42 commented 4 months ago

So I will rename all transport_1.png icons to transport.png and remove the transport*_2.png icons, ok?

OK, thanks! I have updated the code (#385): transport*_2.png icons are no more used and can be safely deleted. Last thing to do in the code after renaming transport*_1.png to transport*.png is to update file names in TGIconManager

Notes: