What is the point of this? What is the user supposed to do when they see this?

Consider updated to a newer version of Mudlet!

I believe we have made incremental changes to what goes into the Xml files but other than some qDebug() reports in some of the XMLimport::readUnknownXXXX() methods there is no safety net should we make/need a big enough change that would throw things awry:

• Adding "attributes" from XML elements can be handled by assuming defaults in new code loading old files and any such new attributes will be ignored by old code loading new files.
• Removing "attributes" from XML elements should it be ever need will also generally be handled by old code taking on whatever defaults that old code had in it, new code will not be looking for those attributes so won't have an issue.
• Adding new "elements" will trigger calls to XMLimport::readUnknownXXXX() methods - some of which will produce qDebug() output in old code loading new format files but this will be suppressed on release builds. Going forward I do have vague future plans for one change in my mind that will need a new element or three...

Problems could come about IMHO if we ever want to remove "elements" from the XML format - like say nuking the misspelled "commandSeperator" that duplicates "commandSeparator" - I think the parser in the code at the moment will enter a QXmlStreamReader::UnexpectedElementError state as we use the default QString QXmlStreamReader::readElementText(ReadElementTextBehaviour behaviour = ErrorOnUnexpectedElement) setting.

As it stands the current code base has nothing to protect against the last case and - unlike the map loading code - does not do any sort of version check on the format for the XML and thus cannot alert the user if something does get changed in the future that does warrant them updating their Mudlet. This code will, going forward, at least tell the user that there is a problem; I did wonder whether we could trigger a version mis-match to a switch to QXmlStreamReader::ReadElementTextBehaviour being QXmlStreamReader::IncludeChildElements and preserve and re-save un-handled elements when the profile is re-saved later though using the final option of QXmlStreamReader::SkipChildElements defensively to silently ignore extra elements but that is something for another occasion.

tl;dr; IMHO this is a minimal implementation to cover warning the user if we need to change the XML used (or at least the "version" attribute to the "MudletPackage" element) to save the Mudlet profile in the future.

I agree on showing a message when the user imports a script for the first time, though the message to the user is too technical in my opinion. Also, in case of modules, would it spam the warning every time or re-save the module with a newer version automatically?

I don't think we need to show any messages in the main output window if the version tag is missing, because should it happen, there is nothing for the user to do anyway, and so we're just spamming them.

With the code as it is - when it is re-saved the version string will be reset to that which the current code uses and understands.

This could cause data loss if the XML is then re-read by a later version which would probably be what created the later version in the first place; on the other-hand: any time something new is added to the format there will have to be something (perhaps an assumed default value/setting) to provide a forward migration route. This is most likely to occur for a developer or someone who uses both the current and some, currently hypothetical future version - so I think that any time there is a mismatch between the version a Mudlet instance uses and what it gets there needs to be a heads-up message. The pain for the user will be that every time they switch from the older xml version using Mudlet instance to the more modern one, they may have to redo/tweak whatever the new features are that the new format supports.

📝 I am quite willing to believe that the wording of the message could need polishing/editing.

As for issuing a message in the absence of a version indication I have traced the "version" attribute being "1.0" back as far as Beta-14 which was before Mudlet 1.0-rc - so it was probably in the very first code to save the Profile as an xml file and which precedes anything in this repository. For it to be absent would actually be an indication that something was amiss, i.e. whatever the user was using was not created by a Mudlet instance, e.g. something edited or crafted by hand, but it could still be valid...! (Oh, or it cud B buggy new kode!)

This PR is likely to not merge cleanly with #373 as they touch common areas in the XML handling classes. I would suggest that this is held, possibly for rebasing until after that PR is merged.

@vadi2 The proposed message would appear as, for example:

 [ ALERT ] - detected file being read is a later version (1.001) than this version
             of Mudlet expects!  It is possible that further issues may be found
             or it may still load and work properly, this is merely an advisory
             message!

Do you have a suggested shorter form for this text, given that the version of Mudlet that we will have immediately after this PR goes in (Version A) cannot know what it will get in the file saved from a future Mudlet version (Version Z) that was not configured to produce a file compatible with Version A! I would be tempted once/if we move away from a default version 1.000 to include that in the message as well which will help if someone has to diagnose a failure...

Such a file might work but need some manual adjustment to compensate for things that the later version no longer includes or it might be totally unusable - c.f. what happens for the more rigid binary Map format compared to the flexibility that XML gives us for this type of data - a Mudlet instance (Version A) HAS to reject later map format versions that it does not understand (from Version Z).

Note: we could always include something in (Version Z with XML format 1.xx) to report what will be a problem in each earlier format (<1.xx) when the user manually chooses to downgrade to save a file from their (Version Z) Mudlet - we could stipulate THAT as a requirement for anything that does need to revise the format to be used...

See #369 - I realised another problem with having this. This actually adds problems for us and doesn't solve any existing ones.

Replying to #369 (comment) in the correct PR:

• 3.0.1 will not be another 2.1 "LTS" release: having features that people add on the side sit unused for years turned out to be a pretty poor idea. We'll be having frequent releases of 3.x and 4.0 will come when i18n is all ready.
• "we should include a version check in the current code so that if we ever have to break the way we structure the XML data in the certain knowledge that it cannot be handled satisfactorily in a way by the current code then we have a clean way to stop it being used and to force the user to seek an updated version of Mudlet." - yes, that is reasonable. But why do we even need to use a version? Mudlet should be able to handle and bail out loading any incompatible files even if they are not nicely tagged with a version for us to understand. Just basic error checking here, innit?

...Mudlet should be able to handle and bail out loading any incompatible files even if they are not nicely tagged with a version for us to understand. Just basic error checking here, innit?

Well it used to throw out a load of qDebug()s about unrecognised entries and their values - but due to an algorithm error it doesn't even do that properly (it reports the unknown entity's name with an empty value on one line and then follows it with a second report with an empty name and the value). However it won't abort/roll-back cleanly IIRC it will just take on what it can and lose the rest, which may or may not be okay... ☹️

The absence of a <MudletPackage ...> in an XML file the user might try to import will stop them loading in a random file that they happen to have lying around their system, and as I checked we have been tagging files with version 1.0 as far as I can see history of (~2008 ish) - I just want to cover our asses should we ever break things in the future.

I think it'll be okay, because I've been switching between 3.0 and 2.1 lots of times and haven't had issues - and we didn't have any complaints about it from the users so far either. I'm now leaning more towards allowing small 'errors' instead of bothering the user for every single detail - it seems to be working out for the modern web applications - rarely do you see error codes or messages anymore, all you instead see is that a button that was supposed to do something didn't do something, for example. Showing an error code to the user wouldnt've solved anything anyway.

Thus unrecognised properties or etc should not be errors or even warnings, it's a given that if you use an older application with newer settings features that those features won't work. That in turn makes XML version numbers kind of moot - if Mudlet can understand it, it should do the best job it can, bothering the user as little as possible, to load the file - exactly what it does right now in fact. If it really can't and stumbles, then abort loading the file and don't save settings when exiting because you haven't loaded anything.

Changing the format of the XML is a different topic for another time.

You posted whilst I was still typing so you hadn't seen my last post before you responded. 😄

That's why you've gotta stop editing comments. I don't get notifications and it's really hard to see edits. Just add another one instead of editing!

It wasn't an edit it was a fresh post - wasn't it - oh bother I'm loosing it!

For sure:

OK, let's put one in in the very extreme case that we'll need to hit the switch sometime. I'm sure you see now though that such a switch does warrant being enabled if we add new properties to the XML or little things like that - things so far have worked fine since the beginning. If it ain't broke, don't fix it sorta thing.

... such a switch does warrant being enabled if we add new properties to the XML or little things like that...

Shirley, that should be doesn't should it not?

Yes, doesn't warrant being enabled, sorry.

Sometimes I forget whether I'm returning to write a new comment after switching back from the "preview" tab to check how a post goes or I've gone to the edit option to grab something like a link or other previously posted element and accidentally gone on to emending it...

oops, emend not "emending"

Closing and replacing with #870 - as code has changed too much to make it easy to resolve conflicts.