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]

This issue has been fixed by v3741 commit 55b75a8

---

- 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.

[Active Colors]

@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.

@Brian,
I have finally prepared for you this skin for test purposes.

Where it says, for example, "Activated skin illustro\Clock\Clock.ini", for that test you need to have the "clock.ini" activated. I put all the buttons to do that for you! Let me know if something is not clear.

As you said, I do hope these things could be "fixed" with just better wording choices.

Some of the bangs output in the log "not active" while others output “not found”. I am not sure what they really mean from the code's perspective, given the confusion on the user’s side. From my user perspective, "not found" implies to me that Rainmeter read the folder and checked if the file and folder exist. Or does “not found” mean “not found among the running skins”? Anyway, my goal was to make you aware of and demonstrate the perceived inconsistenices that hopefully could be resolved with just better wording.

My color coding in the skin:
Green = ok, could be improved
Grey = misleading, unexpected result
Red = critical, possibly a bug

Code: Select all

Rainmeter 4.5.18.3727 (64-bit)
Language: English (1033)
Build time: 2023-07-31 11:58:37
Windows 10 IoT Enterprise LTSC 2021 21H2 (build 19044.1288) 64-bit - English (1033)
Path: E:\Programs\Portable\Utlities\Rainmeter\
SkinPath: E:\Programs\Portable\Utlities\Rainmeter\Skins\
SettingsPath: E:\Programs\Portable\Utlities\Rainmeter\
IniFile: E:\Programs\Portable\Utlities\Rainmeter\Rainmeter.ini

[sl23]

I've had these issues for many years, always wondered if it would get resolved.

I use Rainmeter process monitoring to activate deactivate a couple of skins whilst certain apps are running. If these apps are not running, the skins are activated/deactivated causing two errors.

Will this ever be sorted out? Not a problem, just curious if it's likely to be resolved.

[Brian]

@Brian,
I have finally prepared for you this skin for test purposes.

Thanks for this.

Definitely some wording issues, and some issues with the bangs using the config parameter as a fall back when the ini skin defined in the bang doesn't exist.

I may have to come up with some different options since we probably need to differentiate between different issues.
Like a non-existent file (physically not on the disk), a unavailable file (did not exist when Rainmeter was started, so is not loaded into the internal skin registry), many a locked file? Who knows.

Anyway, this test skin helps, so thank you. Hopefully I can come up with something that works.

-Brian

[Brian]

Ok, I think I have sorted out most of these issues....however, there is at least one issue with your test skin that I did not notice at first.

That issue is most bangs do not accept a File parameter. See !SetOption and !DeactivateConfig (I will talk about this one later). Extra parameters are either ignored or the bang won't work properly (depends on the bang). So in the test skin, these bangs are behaving appropriately since they are ignoring the extra parameter provided.


Outside of that issue, I changed some of the logging messages that *should* help to clarify what the error is. First, I added a check to see if the actual config (and file, if the bang accepts it) exists physically on the disk. There is also a check to see if the config/file exists but it not available to Rainmeter (like if a file is created while Rainmeter is running) in some cases.


The only other issue that I can see involves the !ToggleConfig bang.

First, the documentation is incorrect. Only the Config parameter is required, File is optional. Not sure if that changed at some point (I don't think it did), or if it was just an oversight when creating the docs. We will fix this when a new release comes out.

Second, the underlying code for this bang is really similar to the !ActivateConfig and !DeactivateConfig bangs. To activate a skin, the !ToggleConfig can accept a config and a file - but to deactivate, only the config is needed. So, when the File parameter is provided (but that file doesn't physically exist on the disk), the parameter is ONLY evaluated during the activation code, not the deactivation code. I am not sure what can be done except maybe some documentation on this edge case.

-Brian