Mudlet
New package exporter includes "saved" Variables in exported package
Brief summary of issue / Description of requested feature:
Whilst preparing a test package using the new package exporter I discovered that the packageName.xml file within the packageName.mpackage zip-format archive file contains a <variablePackage> element with some of the variables/tables of variables in the profile. This is despite "Variables" not being something that the package exporter UI offers any control over (that item type is not present therein). As such this represents a potential privacy/data leakage problem as variables can contain information that the user who is exporting a package does NOT wish to share willy-nilly with anyone else!
Steps to reproduce the issue / Reasons for adding feature:
- Create a variable/table with arbitrary value/values.
- Using the "Variable" view in the editor make the variable/table "saved with the profile" by checking the checkbox in the tree view of all the variables.
- Use the package exporter to generate a package containing only a single alias/trigger/key-binding/timer/script (in passing noting that the mechanism that controls which elements get included in the package does not have any interface that includes variables.
- Unzip the created
packageName.mpackagezip archive file. - Examine the extracted
packageName.xmlfile in a text editor (or open it with a web-browser {Mozilla based ones at least} that will display the XML in a structured manner). - Note the present of
<variablePackage>containers with<variable>/<variableGroup>element(s representing numeric or string/table data.
Error output / Expected result of feature
The previous package exporter code which I think was replaced in PR #4885 which preceded Mudlet 4.12.0 could also have this defect - which I think may have originated in code that reused the code that saves the profile data in the background.
Extra information, such as Mudlet version, operating system and ideas for how to solve / implement:
While saving a profile variables do need to be saved but for packages/modules this is undesirable. The mechanism for exporting packages/modules uses a flag (bool) exportItem that is declared in each TXxxx.h file where Xeee is Action/Alias/etc... but which does not appear in the variable type. As it happens the flag is cleared for everything when the package exporter is initiated and set by the selection of Mudlet items in the trees shown in the export dialogue (before ALL the flags are set to be true again at the end of the export). Whilst the TVar class has a (bool) saved flag which determines whether the variable is saved with the profile, this is not intended to be used for the package exporter - though it is being so used currently.
One reason why it would be/is bad to include variables directly in the package is that it seems that a value imported from a package/module will immediately replace an existing variable of the same name already present in the profile (though not be removed should the package/module be removed) - whereas it would be better that any variables that need to be instantiated by a package/module should be managed by an install/uninstall script that can take care to properly merge a variable already extant in the profile with a new package version (with say something like, in Lua profileVariable = profileVariable or wantedPackageVariable) on package install and also to erase/reset the variable when the package in uninstalled...
I'm not so sure we should force people to instantiate variables using code only - for a beginner, being able to have a variable exported with the package would be nice. We should fix this by exposing variables in the exporter UI so people have a choice here to export them or not.
The variable code is largely @Chris7 's - does he have any input on how easily this can be done? At a guess we need to add an (bool) exportItem flag to the TVar class and arrange for them all to be default to false but settable via another treeWidget in the exporter for "variables" and then the (background) XML saving code to consider THAT flag (instead of the saved one) when it is being use to export a package as opposed to saving the profile...
🤔 Humm, the need to include the ancestors as tables to contain any variables that are not directly in the _G table will need to be taken into account I guess?
Going from memory, variables are supposed to be exported, for the exact reasons vadi mentioned -- it's simpler for users to initialize. I used to put variables in the module export code, but never really used the package system so there might be some unintended behavior there.
I just came across this while exporting a package. It saved variables that I ticked the box on to save between sessions that deal with other scripts and weren't at all related to or even used by the scripts/triggers/aliases I was exporting. I create my packages on the same profile I play on, tweaking them and adjusting them as I play until I work out all the bugs...and then export them to be shared, so others who also play on the same MUD might make use of them.
The way I saw it, a package is a group of scripts/triggers/aliases to do a specific function. This package recasts spells on spellcast failures. This package handles your communication window (puts all communication in a separate window/feed). This one logs your quest rewards and displays them so you can track how often you gain each reward. This one saves your player's ingame information for use in other packages. Some packages may have dependencies on others, but a variable from one shouldn't automatically export to the rest of them...especially when it's not used or needed.
Either rely on script-side saving for variables and don't save them to packages at all, or expose variables to be selected in the package exporter. The package exporter shouldn't automatically include every variable that has been checked to be saved between Mudlet restarts.
