On Tue, 27 May 2025 05:20:51 +0200
ichthyo <p...@ichthyostega.de> wrote:

>As part of the changes, I had to understand some aspects of the usage
>made by Yoshimi's core code, and especially the version and compatibilty
>checks turned out somewhat difficult to understand, since this logic
>has evolved over a long time and the implementation was partially
>in Config, partially in the XMLwrapper.
>
>
>First to mention, to test the changed behaviour, one can load an
>existing file, save it back and compare both with a diff tool.
>For this to work, you need to set the "gzip compression level" to 0,
>so that it becomes possible to read the actual XML data on disk.
>
>You will notice some minor changes, like line wrap, and a small
>number of attributes which I have rearranged (waveshaping before
>filter, and the bypass of effects). Furthermore, I have changed
>some toggles in the config storage to bool parameters (which
>is now implemented backward compatible, but obviously is not
>forward compatible, i.e. older versions of Yoshimi will fail
>back to the default settings of these toggles)

As long as this is only config that's changed I have no problem with this.
However, at one time somebody found instruments with bool values stored as "0"
and "1". This was why I did the rather convoluted bool decoding. I don't know if
that is still relevant.

>Please consider these changes as proposal and feel free
>to question and discuss anything...

Mostly this seems fine, and though significantly different, overall it will
definitely be easier to follow. Not having to check all those branch exits are
in place, potentially gets rid of a lot of difficult to find bugs!

>Regarding the compatibility check, I tried to re-create the
>existing behaviour, as can be inferred from code and Git commits.
>But I have not automated it a bit more, and removed the build variables
>MIN_CONFIG_MAJOR |MINOR
>
>However, I am not sure that this behaviour is actually what we want,
>because it will warn on each regular upgrade of Yoshimi. This is
>probably a very defensive approach, because usually we care very
>much to provide a soft migration whenever we introduce changes.
>
>- right now the rule is: warn if major or minor version differs

Unfortunately it is also picking up a Zyn revision number of 3 and thinks it's
the version number. Amusingly it thinks Companion/Ethereal is a Zyn 3 file!

As a point of interest, that Zyn revision number is a mistake on my part, but
I'd already saved some instruments with that number before I realised - it
should never have gone beyond 2!

Also, there were a few small changes in some default values with Zyn 2.5, but I
didn't notice any sound difference in the ones I compared.

>- an alternative rule would be: tolerate one minor version back
>   and on upgrade of the major version, tolerate all minor versions
>   from previous releases, but always warn when loading config
>   from a newer version into an older Yoshimi.
>   This logic could be easily implemented in code

I don't think we need to give any warning when loading an older version. I
can't think of any occasion where we've lost any settings as a result.

Where the file version is newer than the instance version, the warning
should be that re-saving would lose some information.

>- yet another solution would be, to handle compatibility slightly
>   more manually; we could add a new build var COMPATIBLE_CONFIG_VER
>   with a string spec into the CMakeList.txt -- as you may have
>   noticed, I have implemented a parsing function for version
>   specs, and I have added equality and ordering predicates
>   (see Misc/VerInfo.h and the implementation at the top of Config.cpp)
>
>Any thoughts?

I think that if the file has a Yoshimi version and Zyn one, Zyn should be
completely ignored.

If there is only a Zyn version and it's 3 or greater this should have a
significant warning that it is likely to be different, but if it's 2.5 the
warning should be that there *might* be minor differences.

>Another thing which I've notice (but not changed) is the fact
>that Bank::generateSingleRoot() always stores the new files
>in Zyn-format. Was this done intentionally so?

Yes this was intentional, and meant I had to do very little to create it.

Finally, I notice this is slightly slower to startup. Not a big issue, but I
wonder what the cause is.

-- 
Will J Godfrey



_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel

Reply via email to