Improvements to warnings: Activate/Deactivate/ToggleConfig and sending bangs to other skins

[Active Colors]

I certainly don't like the way the log shows the warnings related to the cases like deactivating an inactive skin or sending bangs to an inactive skin.

There were several reports about this as well:
https://forum.rainmeter.net/viewtopic.php?t=36450
https://forum.rainmeter.net/viewtopic.php?t=38328
https://forum.rainmeter.net/viewtopic.php?t=40917


When deactivating an inactive skin, Rainmeter shows: !DeactivateConfig: "illustro\Clock" not active
When sending bangs to an inactive skin, Rainmeter shows: !UpdateMeter: Skin "illustro\Clock" not found

These two log messages are confusing and inconsistent because:

1. !DeactivateConfig: "illustro\Clock" not active → By not active the log means the skin is not running. However, when you try to deactivate a skin that does NOT exist then the error will be again not active. But when you try to activate a skin that does not exist it will show again not active. Logically, if a skin does not exist, then the warning should show not found instead of not active. But there is a confusion with not found too.
2. !UpdateMeter: Skin "illustro\Clock" not found → By not found the log can imply that the config does not exist, but in reality it does exist. For example, when you try using !WriteKeyValue to write into a file that does not exist - it will show that the file is not found: !WriteKeyValue: File not found: E:\Rainmeter\Skins\illustro\Block\Clock.ini. But when you try to update a skin that is not running but does exist - it will show not found too. So, two different cases but the same wording.

I explored other cases and found some more inconsistencies:

• - deactivate inactive config: [!DeactivateConfig "illustro\Clock"] → !DeactivateConfig: "illustro\Clock" not active
  - deactivate inexisting config: [!DeactivateConfig "illustro\Block"] → !DeactivateConfig: "illustro\Block" not active
  - activate inexisting config: [!ActivateConfig "illustro\Block"] → !ActivateConfig: Invalid parameters
  - activate inexisting skin: [!ActivateConfig "illustro\Clock" "Block.ini"] → !ActivateConfig: Invalid parameters
  
  - activate active skin: [!ActivateConfig "illustro\Clock" "Clock.ini"] → !ActivateConfig: "illustro\Clock" already active
  - activate active config: [!ActivateConfig "illustro\Clock"] → Nothing
  
  - toggle inexisting config: [!ToggleConfig "illustro\Block"] → !ActivateConfig: Invalid parameters (why !ActivateConfig if it is !ToggleConfig?)
  - toggle inexisting skin within inactive skin: [!ToggleConfig "illustro\Clock" "Block.ini"] → !ActivateConfig: Invalid parameters
  - toggle inexisting skin within active config: [!ToggleConfig "illustro\Clock" "Block.ini"] → Deactivates the active skin if the config exists!
  
  - send bang to inactive config: [!SetOption meterDay FontSize "20" "illustro\Clock\"] → !SetOption: Skin "illustro\Clock" not found
  - send bang to inactive skin: [!SetOption meterDay FontSize "20" "illustro\Clock\" "Clock.ini"] → !SetOption: Skin "illustro\Clock" not found
  - send bang to inexisting config: [!SetOption meterDay FontSize "20" "illustro\Block\"] → !SetOption: Skin "illustro\Block\" not found
  - send bang to inexisting skin within inactive config: [!SetOption meterDay FontSize "20" "illustro\Clock\" "Block.ini"] → !SetOption: Skin "illustro\Clock\" not found
  - send bang to inexisting skin within active config: [!SetOption meterDay FontSize "20" "illustro\Clock\" "Block.ini"] → Sends bang to the active skin if the config exists!

The following steps could be taken to resolve the inconsistencies and make the logs better:
1. The hardest part: Make Rainmeter actually verify if a config folder or skin file actually exist.
2. Then, make the wordings to be consistent. Make it clear when a config/skin is "not active" and when it is "not found".
3. Make "not active" to be a Notice because if a skin/config exists but is inactive, there is nothing wrong. But make "not found" to be an Error because if a specified config/skin does not exist, there is something indeed wrong.

And only then, additionally, there could be a skin-level option that would let you hide the "not active" errors. For example in a skin's [Rainmeter] section it could be something like WarnOnNotFoundConfig=0.


Overall, if Rainmeter learns whether a file or config actually exist, it will be able to judge better whether something is a fatal error or not, and provide a better logs.
What do you think?

Rainmeter 4.5.13.3632 (64-bit)
Language: English (1033)
Build time: 2022-03-23 15:23:40
Windows 7 Professional 64-bit (build 7601) Service Pack 1 - English (1033)
Path: C:\Program Files\Rainmeter\
SkinPath: E:\Custo\Rainmeter\Skins\
SettingsPath: C:\Users\Focus\AppData\Roaming\Rainmeter\
IniFile: C:\Users\Focus\AppData\Roaming\Rainmeter\Rainmeter.ini

[Jeff]

- toggle inexisting config: [!ToggleConfig "illustro\Block"] → !ActivateConfig: Invalid parameters (why !ActivateConfig if it is !ToggleConfig?)

Reviving this topic cause I ran into this problem as well too when trying to deactivate a skin, and it was a head scratcher looking at the docs page saying File is (required) when I can see just above deactivate dosen't need one.
Digging into the code, these lines are the culprit

Code: Select all

void CommandHandler::DoToggleSkinBang(std::vector<std::wstring>& args, Skin* skin)
{
	if (args.size() >= 2)
	{
		Skin* skin = GetRainmeter().GetSkin(args[0]);
		if (skin)
		{
			GetRainmeter().DeactivateSkin(skin, -1);
			return;
		}

		// If the skin wasn't active, activate it.
		DoActivateSkinBang(args, nullptr);
	}
	else
	{
		LogErrorF(skin, L"!ToggleConfig: Invalid parameters");
	}
}

For some god forsaken reason, from what I understand in PseudoCode, it's written like this.

• If 2 arguments (Config and Variant) then
  • If Skin Exist
    • Deactivate Config
  • Else Activate Config
• (Else Error :P)

Let's do some thinking, !ActivateConfig can have one or two parameters, and !DeacitvateConfig can have only one parameter, it would make more sense if the code was written like:

• If 2 arguments (Config and Variant) then
  • If Skin Not Exist
    • Activate Config (with Variant)
• Else If 2 arguments and Skin Exist OR If 1 argument (Config) then
  • If Skin Exist
    • Deactivate Config
  • If Skin Not Exist
    • Activate Config
• Else Error :)

This would at least be what I expected, or more specifically what I desire, it just struck me in awe that's all, """theoretically""" it dosen't seem that hard to implement.

EDIT: Ok, I can kinda see why nobody wants to touch this, the logic is there but the more I read, the more I can't wrap my head around what I wrote. Is it even possible? (to at least write more ellegantly)

EDIT 2: I tried to wrap my hand around it, I have never written a line of CPP in my life and this is just assumptions from what I see in the snippet I sent above, technically the fixed code should just be just:

Code: Select all

void CommandHandler::DoToggleSkinBang(std::vector<std::wstring>& args, Skin* skin)
{
    if (args.size() == 2)
    {
        Skin* skin = GetRainmeter().GetSkin(args[0]);
        if (!skin)
        {
            DoActivateSkinBang(args, nullptr);
        }
    }
    else if ((args.size() == 2 && GetRainmeter().GetSkin(args[0])) || args.size() == 1)
    {
        Skin* skin = GetRainmeter().GetSkin(args[0]);
        if (skin)
        {
            GetRainmeter().DeactivateSkin(skin, -1);
        }
        else
        {
            DoActivateSkinBang(args, nullptr);
        }
    }
    else
    {
        LogErrorF(skin, L"!ToggleConfig: Invalid parameters");
    }
}

I hope I got it right first try, I don't know how to build any app, whoever wants to contribute is free, having a major guide here!

[Brian]

@Jeff:
Thanks for reminding me of the issue with !ToggleConfig. The issue has been around for over 11 years since we made the 2nd parameter of !ActivateConfig bang optional.

A fix for this specific issue will be in the next release.

---

@ActiveColors:
I will look into these inconsistencies a bit harder when I get some time. It might save me some time to have a small set of skins to test with.

The tricky part is deciding if we really want to test if the config exists physically on the disk...or if it exists in the internal skin registry (the list of skins built when Rainmeter first starts). I mention this because there could be a lot of read access in a short amount of time (like if an invalid !ActivateConfig bang resides in an OnUpdateAction with a low Update value). Maybe it is just a "wording" issue.

-Brian

[Yincognito]

Personally, I would just silently ignore ALL these warnings and errors, similar to how Rainmeter reacts when there are multiple sections or options having the same name, since most of the times they occur because of bad coding (a bang that shouldn't be there) or bad skin structure (a file that isn't there), and the rest of the times they occur because of intentionally and valid placed bangs (like when you want to make sure a skin is deactivated before activating it, even though it might already be deactivated or even missing). I would do the same with the "errors" about missing images.

Similar example: would you like errors to be thrown when disabling an already disabled or non existing measure? Or enabling an already enabled or non existing one? Me neither, the same with skins. It's up to me to realize whether this was a mistake or something intentional.

Just my two cents, I know most of the users or developers think otherwise. I just loved the once vast ability of Rainmeter to do its thing regardless of irrelevant details that should be the exclusive concern of the skin designer. This area is already way too verbose for me.

My skin suite abounds of testing ground for this, but it's true that the environment is not simple, so they might not be suited for easy testing (though they might for a real world, in the wild scenario).

[balala]

At least some of these messages (however obviously not all of them) can be avoided by using groups. For instance the message we get when executing the [!DeactivateConfig "illustro\Clock"] bang when the proper skin is not active can be avoided by including the illustro\Clock\Clock.ini skin into a group (let's name this group Group=illustro) and replacing the [!DeactivateConfig "illustro\Clock"] bang by [!DeactivateConfigGroup "illustro"].
Obviously this might be a solution just for a few of the existing bangs.