Hi Will, Hi Kristian, thanks for the review and feedback! So overall, it seems the changes are playing out well. You brought up some points; to follow up I have just pushed a commit to my mxml branch to improve the compatibility handling. On 28.05.25 10:38, Will Godfrey wrote:
Unfortunately it is also picking up a Zyn revision number of 3 and thinks it's the version number.
And this is indeed a bug; I had it on my list but then forgot to add the additional check if a yoshimi verison number is present, because in that case the file was in fact generated by Yoshimi and we need not bother about any Zyn version numbers. This is now fixed. On 28.05.25 10:04, Kristian Amlie wrote:
I think I prefer the first option, except that for minor version I would not warn on just any difference, only when the loaded minor version is higher than the software minor version, since this is the only "dangerous" case. When bumping the minor version, things are supposed to stay backwards compatible, so the opposite case should be safe.
On 28.05.25 10:38, Will Godfrey wrote:
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.
These proposals together seem to indicate a good path to handle this issue. One additional aspect however is that we try to /migrate/ an incompatible config automatically by re-saving. This might not be a good idea in case of a newer config loaded into an older version. Taking those points into account, I now did the following changes - implement a more fine-grained compatibility logic. See Config::is_compatible(VerInfo) * only revision differs => compatible * other version is newer => incompatible * both have same major: silently accept any lower minor * when current-Yoshimi is a major-bump: accept 1 major back * else mark any earlier major as incompatible - only take any action if this check cadence indicated incompatibility - for the migration, we now additionally check if the loaded config was from a newer version. In this case we warn only, but don't automatically save it any more - Improved the message at GUI start-up to include the actual version numbers involved, and to differentiate if the config was auto-saved, or if (for newer config) we should even warn to be careful with saving. On 28.05.25 10:38, Will Godfrey wrote:
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.
Something along these lines is a widely used practice. If I recall correct, in some of the newer C++ versions there will even be added a similar check function in the standard lib. Basically, the transport-encoding of bool values in textual data is not standardised (as opposed to the representation of decimal numbers). It's thus a very good idea to act defensive and fault tolerant.
Finally, I notice this is slightly slower to startup. Not a big issue, but I wonder what the cause is.
Haven't noticed that, but I'll do a comparison with two release builds. -- Hermann _______________________________________________ Yoshimi-devel mailing list Yoshimi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
