[FIXED] FadeDuration below 0 prevents skins from unloading

[Jeff]

This issue has been fixed by v3759 commit a8eb1a1

I had to remove the alternative titles section since it could confuse people searching on the forums :(

---

How to reproduce

1. Set any skin's FadeDuration to below 0 (I went with -1)
   (you can also alternatively do [!FadeDuration "-1"])
2. Save the Settings file
3. Try to unload a skin
4. Skin will not unload, context action options also don't appear anymore, Manage Rainmeter says the skin is unloaded

Example
https://i.imgur.com/3LD1VDb.mp4

Why it happens and potential place to fix
I tried to look at the code but I wasn't able to put my hand on why it happens, the code deactivating a skin should be found in here. Surprisngly, Skin::Deactivate() isn't the one that Disposes of the skin in memory (I actually can't tell how it gets deactivated or what ~Skin() means), as for stuff inside the Deactivate function, GetRainmeter().RemoveSkin(this); and GetRainmeter().AddUnmanagedSkin(this); both get executed correctly, so I think it's safe to assume m_State = STATE_CLOSING before them also gets executed and the line before, if (m_State == STATE_CLOSING) return; will just make everything below where it's placed not work, so when the 2nd skin gets loaded over, the first one is gonna make it impossible to ever receive meaningful input in that function.

Whatever, as for potential fixes, this problem can also be triggered by [!FadeDuration "-1"]) (thanks to Skin::UpdateFadeDuration()), so we should be a bit cautious
I think modifying the logic to be max(a,b) on the following lines should be enough though without messing stuff up
Line 140: SetTimer(m_Window, TIMER_DEACTIVATE, max(0, m_FadeDuration) + 50, nullptr);
Line 4221: m_FadeDuration = max(0, m_NewFadeDuration);
Line 2254: the line m_FadeDuration = max(0, m_FadeDuration); should be added after line 2254

Fixes
Close and start Rainmeter again

Pros/Cons

• This is actually quite fun
• Especially the multiple windows, sadly they don't share the same pointer to measures between them, the windows create the measures for each window
• (Technically) Memory leak :((((((

[Yincognito]

Just to add some potentially related things to it: I don't use negative fade duration for my custom tooltip skins, but I did experience "half-closed" skin states with them (e.g. unloaded in the Manage Rainmeter window, but loaded according to the "bullets" from the context menu, with the latter apparently getting it right), and also the monitor variables being set to the primary monitor instead of the second monitor right after loading and redrawing them (when I tried a multi monitor setup with my smartphone and SpaceDesk), so these areas do have some bug potential IMHO.

[SilverAzide]

Surprisngly, Skin::Deactivate() isn't the one that Disposes of the skin in memory (I actually can't tell how it gets deactivated or what ~Skin() means),

If you are curious, ~Skin() is the Skin class destructor. It does the opposite of the class constructor, and in theory this would be final place where the resources allocated by the class would get deallocated, memory freed, etc.

[Brian]

@Jeff:

Thanks for your bug report and fixes, they helped.

This wasn't technically a memory leak because the skin was still in the fade duration loop while closing (this is the "unmanaged skin" in the code). The problem was the option never accounted for negative numbers while in the loop, so it never ended.

Anyway, your fixes were the best option (except the one in the SetTimer function, which wasn't needed with the other fixes).

Thanks.

-Brian