[Client] Modify preferences if niu & in use times overlap

[Vulpine05]

Partially fixes #4939

Description of the Change
This commit checks for the condition stated in #4939 when global preferences are loaded to the client. If the condition occurs, the 'in use' suspension period is set to 0 and the run CPU/GPU when active are set to true.

Alternate Designs
Instead of 'in use' being set to zero, the suspend computing after idle for x minutes could have been set to zero. However, I felt that if the global preference settings were made by accident, the user would not notice the setting change. The computer is not being used at that point in time, so how would you know?

Release Notes
Fixed indefinite suspension when suspension times were not set correctly in global preferences.

[AenBleidd]

I don't feel OK modifying settings that were added manually by users.
In this case values are technically (but not logically!) correct, so I believe client should just generate a warning in the log while loading config.
@Vulpine05, please change this PR.
Thank you in advance

[Vulpine05]

@AenBleidd, understood. I assume the concern is changing a valid preference setting to another valid value? In other words, what I mentioned in #4916 is still worthwhile. I just want to double-check.

The error log idea is a good idea, but I am concerned it may go unnoticed, either in this PR or #4916. I was thinking the warning would be more noticeable if it was generated as a notice, similar to when there is a problem with an app_config file (which also is user modified). The only downside I see is if there are a lot of errors in the global_prefs_override file, then the notices tab gets flooded, but I think that would be a very unusual case. Thoughts?

In either case, I think it makes sense to close this PR and open a new one to implement a messaging system (notice or error log) for all settings. Sounds good?

[davidpanderson]

If you generate a message with
msg_printf(0, MSG_USER_ALERT, ...
it will be shown as a notice as well

[Vulpine05]

If you generate a message with msg_printf(0, MSG_USER_ALERT, ... it will be shown as a notice as well

Looks like that should do it, but I'm wondering if msg_printf_notice is better? I would try both, but regardless of which function I try, I get the following error when building boincmgr:

(snip)\client\client_msgs.h(77,1): warning C4005: '_': macro redefinition
(snip)\3rdParty\Windows\vcpkg\installed\x64-windows-static\include\wx\translation.h(39): message : see previous definition of '_'
prefs.obj : error LNK2019: unresolved external symbol "void __cdecl msg_printf(struct PROJ_AM *,int,char const *,...)" (?msg_printf@@YAXPEAUPROJ_AM@@HPEBDZZ) referenced in function "public: int __cdecl GLOBAL_PREFS::parse_override(struct XML_PARSER &,char const *,bool &,struct GLOBAL_PREFS_MASK &)" (?parse_override@GLOBAL_PREFS@@QEAAHAEAUXML_PARSER@@PEBDAEA_NAEAUGLOBAL_PREFS_MASK@@@Z)
(snip)\win_build\Build\x64\Debug\boincmgr.exe : fatal error LNK1120: 1 unresolved externals

Please note, there isn't a problem when I build the client. Based on my limited knowledge, it looks like the following line in client_msgs.h is in conflict with wxwidgets:

boinc/client/client_msgs.h

Line 77 in 572f7c0

#define _(x) "_(\"" x "\")"

Its a little too abstract for me, I would think this line needs to be edited, but I'm not sure what. Can anyone help? I just pushed a commit to show where I am at.

[AenBleidd]

This is not correct include. You call client function from the library that is not correct. You should refactor your code to catch this on client side and send message from the client.

P.S. Basically this error tells you that function that you call is declared but not implemented in the binary where you call this function.

[davidpanderson]

do the check / message in
client/cs_prefs.cpp: void CLIENT_STATE::print_global_prefs()

[Vulpine05]

do the check / message in client/cs_prefs.cpp: void CLIENT_STATE::print_global_prefs()

The check already happens for other preferences in prefs.cpp: GLOBAL_PREFS::parse_override. For example,

boinc/lib/prefs.cpp

Lines 539 to 544 in 572f7c0

	if (xp.parse_double("niu_max_ncpus_pct", niu_max_ncpus_pct)) {
	if (niu_max_ncpus_pct <= 0) niu_max_ncpus_pct = 100;
	if (niu_max_ncpus_pct > 100) niu_max_ncpus_pct = 100;
	mask.niu_max_ncpus_pct = true;
	continue;
	}

I would think this is the place to do a sanity check for all preferences. I am planning on adding checks for the other preferences at a later date.

As for printing the message, it makes sense to print in cs_prefs, but I'm wondering if it makes this a bit complicated. I would have to create a message vector to pass from one function to the next. I tried this earlier and I felt that I was going down a rabbit hole adding that parameter throughout the code. Assuming that problem can be solved, then it probably would be best to have the errors load up in order, which is another point of data that the message vector (or another structure) would have to store. I feel printing the error message in its place makes the process a bit streamlined.

I'm just explaining my point of view. If you really want it in cs_prefs, I'll see what I can do.

[AenBleidd]

The place in code you mentioned is the one where file is parsed. But you can make verification of the parsed data in the place where it's actually used, in client, and print the message from there. This is logically correct.

[AenBleidd]

Hey, @Vulpine05, any plans to continue working on this PR?

[Vulpine05]

@AenBleidd I stopped working on this PR only because I thought #5050 would be its replacement after it was developed more than the draft that it is currently. However, now that I am reviewing this PR and #5050, I am not so sure.

The point of this PR is to adjust a setting that although technically correct, is not logically correct (as you stated in a previous comment, very good point). I'm fine with the suggestion to provide a message to the user as well. However, now that I look at the rest of the preferences, I'm wondering if we are being consistent for the remaining preferences. This concern is brought up in #4916, as with the example of suspend_cpu_usage, it can be written as a negative number. Although a negative number is technically correct (for a double), it is not logically correct for a period of time. Similar situation for percentages, which are corrected in GLOBAL_PREFS::parse_override. Why is it okay to change a percentage value, but not a period of time? There are a few preference variables that I feel are related to this same question.

So now I think I have a few questions before moving forward:

1. When should it be okay to modify a preference if global_prefs(_override) in parse_override if the value is parsed correctly?
2. Where should the preference be modified? parse_override, or in a different function?
3. If the preference is modified, or if the preference creates an undesired situation, should the Manager provide a message?

If you want me to move this to a discussion I can.

[Vulpine05]

Closing in favor of #5270.