HEnquist / camillagui

GNU General Public License v3.0
9 stars 1 forks source link

Possible to disable setting the default config from the gui #42

Open bitkeeper opened 3 years ago

bitkeeper commented 3 years ago

@JWahle regarding the new functionality to set the default active config from the camillagui; would it be possible to prevent this action with an option in the gui-config.yml? Only setting showing is just fine.

image

Case: In moOde the active is already set in moode config pageconfig, which is also stored in the db. With the new set default config from the camillagui this will get out of sync. Which is a little bit confusing for users.

JWahle commented 3 years ago

This should be easy to implement, but I'm not quite sure, if we should. To me this sound like a workaround for another problem. Why is it necessary to store the active config in the db? Why can't the moOde config page read from the symbolic link?

bitkeeper commented 3 years ago

moOde stores all it settings in the database and generate the configurations for arround 25 other applications. In this way the most settings are in a central location. Which make it more easy to migrate and backup. This doesn't come from me, that is just how moOde works from the beginning and to which I need to conform.

In the past there was no default config setting in camillagui, which required that moOde implemented this functionality. So there was no problem or workarround at all until you changed the behaviour/workflow. Don't get me wrong I like very much like the improvements you make on the gui :-), but please keep in mind every time you make a change in behaviour(or workflow) what the impact that can be on existing integrations.

So if you are willing to do me favor by providing the requested option, I would really appreciate it ! :-)

JWahle commented 3 years ago

So if you are willing to do me favor by providing the requested option, I would really appreciate it ! :-)

I will continue to provide the features, you need to make this work nicely in moOde, as fast as I can. I also understand, that this breaks a workaround, that you had to implement, because it was not implemented in CamillaGUI. The reason I am asking is, because I thought of implementing an "Instant mode" like in EqualizerAPO. This would make the CDSP state the same as the GUI state, without the user having to save. It should also persist across restarts. If the backend can't change the active config, then "Instant mode" can never be fully working in moOde. Thus, if by any means possible, I would like to be able to change the active config in the backend.

If it is possible to make the symbolic link the single source of truth, I'd prefer that. Maybe you can hook into the backup/restore mechanism and read/write the active config from there, instead of using the db? If everything else fails, I could add a setting to camillagui.yml, where you could register a hook script, that is called whenever the active config changes. The script could then update the db entry.

bitkeeper commented 3 years ago

Let's compromise: If you at least for the upcoming gui release (that matches with cdsp 0.5.0/0.5.1) can provide the requested setting. That gives us the time to see how we can improve the moOde side on a structural bases for subsequent release.

JWahle commented 3 years ago

I don't see, how this is a compromise, if I implement it exactly the way, you first asked. Can you please comment on my technical propositions?

bitkeeper commented 3 years ago

The comprise is that we will try to do the integration in moode more in the way how you prefer. But first I want to have cdsp 0.5.1b + the new version of the gui up and running as it is. Then take time to do the integration, concerning the active config in a 'right' way.

So you buy me time and I will make changes later on. Without forcing me to make the changes righ away.

HEnquist commented 3 years ago

Can't you just comment out the few lines that set the active config in the backend that ships with moOde?

Everything here is still in heavy development, so breaking changes will unfortunately happen from time to time. Hopefully they will become less and less frequent.

On a slightly different topic, we should think about some way to provide the same functionality without using symlinks. They work well on linux and macOS, but it's not really an option on windows. For example, what if we start camilladsp with a new option, camilladsp --link textfile_with_path_to_a_config.txt?

bitkeeper commented 3 years ago

Guys you are make more fuss about it then it is worth; I only request an option to disable the buttons on file manager tab to make a config active, that is it.

This option is in no way block any development at your side. Just like the four other already present options to customize the gui from moode. And it will prevent a support nightmare for on the forum for me.

HEnquist commented 3 years ago

... could add a setting to camillagui.yml, where you could register a hook script, that is called whenever the active config changes. The script could then update the db entry.

I like this idea! Let's say we add hooks for setting and getting the active config, that if defined would be run by filemanagement.py/set&get_active_config() instead of reading/writing the symlink. That would make it possible to handle all kinds of systems and integrations. I don't know how you interact with the moOde database, but I'm guessing it would be possible to use this to store the setting there.

JWahle commented 3 years ago

So you buy me time and I will make changes later on. Without forcing me to make the changes righ away.

@bitkeeper are we in a hurry? I thought moOde 7.2.0 is just released and we still have some time until the next release.

HEnquist commented 3 years ago

I liked the script idea so much that I implemented it right away. It's in the final050 branch of the backend. @bitkeeper can you use this to get and set the active config from moOde?

JWahle commented 3 years ago

@HEnquist the last changes on the final050 branch include a workaround for the symbolic link creation on windows. If this is such a problem on windows based systems, I would prefer simply writing to/reading from a plain text file. This would keep the program code and user configuration simple. With a little bit of shell magic, one can probably still use this file directly to call CamillaDSP.

HEnquist commented 3 years ago

Symlinks on windows is a no-go. They do exist but enabling them is too complicated for the average user. What is the problem with allowing running scripts? I think it's a nice very general solution that can help integrate with other systems we haven't even thought about yet.

JWahle commented 3 years ago

I am fine with the hook to run scripts. I am actually happy, that you implemented it :) My remark was not about that, but about the workaround for windows systems.

HEnquist commented 3 years ago

Ok I see! I put in the windows check to avoid the ugly exceptions you get if you try to create symlinks without setting up the weird permissions stuff that is needed for that to work.

HEnquist commented 3 years ago

Played a little on a windows box. This way a filename can be read from a file into a variable and used in another command:

SETLOCAL
set /p camillaconf=<config.txt
echo %camillaconf%
ENDLOCAL

We could include this script with the backend, and read/write the config name to a text file when on windows (instead of just skipping like it's doing now). I also looked into reading and creating windows shortcuts (.lnk) with shell commands, but there is no built in tool for that.

JWahle commented 3 years ago

Nice. I think with 3 scripts (or one script, that supports all 3 options) to set, get and run CDSP with the active config, we should be fine here. The active_config script could be called like this:

I also thought about removing the symbolic link on unix systems and using scripts there too. The backend would choose which set of scripts to use by itself, based on the running OS. (The symbolic link could also be kept as an implementation detail of the scripts - we just would not document its behavior anymore in the README.md) Then it would not be necessary for the user to configure anything for basic usage. An advanced user could still modify the scripts.