[implemented] Name change notifications for addons

[sirinsidiator]

Addons would profit immensly from knowing when their saveData has to be updated because of display name or character name changes.
The best solution I can think of is an EVENT_DISPLAY_NAME_CHANGED(oldDisplayName, newDisplayName) and EVENT_CHARACTER_NAME_CHANGED(oldCharacterName, newCharacterName).
They should be fired right after EVENT_ADDON_LOADED and once they are over, the save data needs to be written to disk.

Please also add an autosave feature that flushes all changed save data to disk every few minutes

[votan]

Saved variables are based on character name. Renaming will create "zombies" like deleted chars aswell.
So, in addition to the previous post, it would be nice to know all currently available names.

Maybe by making RequestCharacterList in-game private and you fill a "read-only" list for us

[Ayantir]

I would personally prefer a

local oldName = GetOldCharName()

at 1st login after a rename. This function will remain valid for all session after a rename.

like GetIsNewCharacter()

[haggen]

The NameChange stuff would help but wouldn't really solve the problem. We should be tracking characters by a CharacterID instead of its name, and now we see why.

[votan]

Originally Posted by haggen The NameChange stuff would help but wouldn't really solve the problem. We should be tracking characters by a CharacterID instead of its name, and now we see why.

Yes, exactly. Basically this exists already in form of

RequestCharacterList()
local name, gender, level, championPoints, class,
 race, alliance, id, locationId, needsRename =
 GetCharacterInfo(i)

But pre-game only.
If ZOS has concerns to make them public, because RequestCharacterList() could be used too often or somewhat, they can make them "private" now. Giving us just the information they want us to. Which could include an id.
Maybe adding an additional return value: *nilable* formerName

[ZOS_ChipHilseberg]

The tricky part is that it's possible to change a character name and then quit the client, so we'd need some sort of persistence here. This is a problem we're working on solving, and hopefully in a way that every addon that uses saved vars doesn't have to roll their own upgrade code to handle this (though more complex or non-standard uses will have to). Right now the plan is to add some automatic functionality to ZO_SavedVars:New that can be opted out of.

[votan]

Originally Posted by ZOS_ChipHilseberg The tricky part is that it's possible to change a character name and then quit the client, so we'd need some sort of persistence here. This is a problem we're working on solving, and hopefully in a way that every addon that uses saved vars doesn't have to roll their own upgrade code to handle this (though more complex or non-standard uses will have to). Right now the plan is to add some automatic functionality to ZO_SavedVars:New that can be opted out of.

So, basically you are saying, that addons with non-standard saved variables need this information anyway

And more tricky: The old/former name must be persisted anyway (until next renaming), because you don't know, if addons (standard and non-standard) were enabled since when.
/edit: What I mean is: If you implicit convert variables in ZO_SavedVars:New, you must be able to do this any time after renaming.

And what if the user wants to be a really bad guy and renames a char to the old name of another char?

[sirinsidiator]

In my addons I do not use ZO_SavedVars, so I am definitely in need for an alternative way to do this.

If persistence is a problem, maybe you can just put a "history" somewhere into ZO_Ingame.lua.
When the name of a character is changed in the pregame scene, or a character is deleted, or the displayName is changed you write that information to disk in pregame and once the client was ingame all addons can check what has changed and upgrade their vars accordingly.

It could look like this:

accountHistory = {
  {ACCOUNT_EVENT_CHARACTER_CREATED, "name", <timestamp>},
  {ACCOUNT_EVENT_CHARACTER_NAME_CHANGED, "oldname", "newname", <timestamp>},
  {ACCOUNT_EVENT_CHARACTER_RACE_CHANGED, <timestamp>},
  {ACCOUNT_EVENT_CHARACTER_LOOK_CHANGED, <timestamp>},
  {ACCOUNT_EVENT_CHARACTER_DELETED, "name", <timestamp>},
  {ACCOUNT_EVENT_DISPLAYNAME_CHANGED, "oldname", "newname", <timestamp>},
}

It should be read-only so that an addon cannot change the information. Race and look change is probably not necessary, but wouldn't hurt either.

EDIT: Maybe instead of providing it via normal saveData you could also just return it from a function called GetAccountHistory.
In addition to timestamps you could give the events an id and if it is passed to GetAccountHistory it would only return events that come after that id.

EDIT2:
The upgrade code would then look something like this:

for i=1, #accountHistory do
    local id, type, timeStamp, name, newName = unpack(acountHistory[i])
    if(type == ACCOUNT_EVENT_CHARACTER_NAME_CHANGED) then
        saveData[GetDisplayName()][newName] = saveData[GetDisplayName()][name]
        saveData[GetDisplayName()][name] = nil
    elseif(type == ACCOUNT_EVENT_CHARACTER_DELETED) then
        saveData[GetDisplayName()][name] = nil
    elseif(type == ACCOUNT_EVENT_CHARACTER_CREATED) then
        saveData[GetDisplayName()][name] = ZO_DeepTableCopy(defaultData)
    elseif(type == ACCOUNT_EVENT_DISPLAYNAME_CHANGED) then
        saveData[newName] = saveData[name]
        saveData[name] = nil
    end
    lastId = id
end

(I changed the order for the timestamp to make it easier to get the right arguments)

[ZOS_ChipHilseberg]

So here's what we were able to do for this issue:

We added a function to get a unique identifier for the current character. It's a 64 bit number represented as a string. We have also added a new ZO_SavedVars constructor named NewCharacterIdSettings which will store the settings under the current character id instead of the name. It will also look to see if there is a setting table under the character name and automatically copy that to the character id key, then delete the character name copy. For most addons it should be sufficient to replace ZO_SavedVars:New with ZO_SavedVars:NewCharacterIdSettings to handle name changes. This has been done to all stock UI saved vars. Finally, we added an automatically generated key at the namespace level called "$LastCharacterName" which will hold the last name the character with that id had so the saved var files are a bit easier to read and modify manually. Let me know if there are any questions or concerns.

[haggen]

Originally Posted by ZOS_ChipHilseberg So here's what we were able to do for this issue: We added a function to get a unique identifier for the current character. It's a 64 bit number represented as a string. We have also added a new ZO_SavedVars constructor named NewCharacterIdSettings which will store the settings under the current character id instead of the name. It will also look to see if there is a setting table under the character name and automatically copy that to the character id key, then delete the character name copy. For most addons it should be sufficient to replace ZO_SavedVars:New with ZO_SavedVars:NewCharacterIdSettings to handle name changes. This has been done to all stock UI saved vars. Finally, we added an automatically generated key at the namespace level called "$LastCharacterName" which will hold the last name the character with that id had so the saved var files are a bit easier to read and modify manually. Let me know if there are any questions or concerns.

This is awesome! Thank you very much.

[ZOS_ChipHilseberg]

So totally kidding on that first post. I restructured it today to keep using the names as keys. I added in a NAME_CHANGE object that will tell you if the current character's name has changed since the last time you logged into it on that PC and also what the old name was. Saved Vars will also automatically copy from the old character name settings to the new one on saved var creation. This should be as robust as the previous solution while allowing the table structure to stay the same and not have have unreadable character id keys.

[votan]

Originally Posted by ZOS_ChipHilseberg So totally kidding on that first post. I restructured it today to keep using the names as keys. I added in a NAME_CHANGE object that will tell you if the current character's name has changed since the last time you logged into it on that PC and also what the old name was. Saved Vars will also automatically copy from the old character name settings to the new one on saved var creation. This should be as robust as the previous solution while allowing the table structure to stay the same and not have have unreadable character id keys.

aha. Only since the last time? Or since last rename? Addons may not be active on the first time after renaming.

[ZOS_ChipHilseberg]

Originally Posted by votan aha. Only since the last time? Or since last rename? Addons may not be active on the first time after renaming.

True enough. It's all the same for the internal UI. Which would you folks prefer?

[votan]

Originally Posted by ZOS_ChipHilseberg True enough. It's all the same for the internal UI. Which would you folks prefer?

I would say "last rename". Maybe a timestamp of renaming?

[sirinsidiator]

Originally Posted by ZOS_ChipHilseberg True enough. It's all the same for the internal UI. Which would you folks prefer?

From the two ways that you have brought up, I would prefer the unique identifier.

I don't think just giving us the previous name is enough. What will happen when a user renames the character, loads the game, quits the client via alt+f4 and loads the game again? Or when he disables an addon, renames, loads, renames again and then enables the addon?

Giving us the unique identifier is a good way to make sure that we have the right character, regardless of how often it is renamed, but using it as the key in the save data may be inconvenient for users that want to edit the save data from hand and putting the character name in the table is not really ideal as it could break an addon that doesn't expect the new entry in the table. Maybe you could extend the save data serialization so it can put comments into the file and add the character name there? That way it could be in the same line as the table key and would not interfere with existing addons. I am not sure how the API for that would look though.
Maybe have a method to read and write comments for a key in a table? GetSaveDataComment(table, key), SetSaveDataComment(table, key, comment)?
The case where a player disables an addon before the conversion to the new identifier, then renames and afterwards reactivates the addon would still remove the save data, but I think it could be neglected.

I still believe having the account history would be a better way of solving this problem, but identifiers are also nice. Maybe we could also get them for another player's characters? Some addons save things about other chars and would not be able to follow renames, which would result in a loss of data.

[haggen]

Originally Posted by sirinsidiator From the two ways that you have brought up, I would prefer the unique identifier. I don't think just giving us the previous name is enough. What will happen when a user renames the character, loads the game, quits the client via alt+f4 and loads the game again? Or when he disables an addon, renames, loads, renames again and then enables the addon? Giving us the unique identifier is a good way to make sure that we have the right character, regardless of how often it is renamed, but using it as the key in the save data may be inconvenient for users that want to edit the save data from hand and putting the character name in the table is not really ideal as it could break an addon that doesn't expect the new entry in the table. Maybe you could extend the save data serialization so it can put comments into the file and add the character name there? That way it could be in the same line as the table key and would not interfere with existing addons. I am not sure how the API for that would look though. Maybe have a method to read and write comments for a key in a table? GetSaveDataComment(table, key), SetSaveDataComment(table, key, comment)? The case where a player disables an addon before the conversion to the new identifier, then renames and afterwards reactivates the addon would still remove the save data, but I think it could be neglected. I still believe having the account history would be a better way of solving this problem, but identifiers are also nice. Maybe we could also get them for another player's characters? Some addons save things about other chars and would not be able to follow renames, which would result in a loss of data.

/sign that, but my conclusion is that identifiers is the best option. It might be painful right now but safe in the long term, and since we don't follow strict standards and some of us are casual coders I'd take the safer bet and avoid future complications.

[votan]

Originally Posted by sirinsidiator From the two ways that you have brought up, I would prefer the unique identifier. I don't think just giving us the previous name is enough. What will happen when a user renames the character, loads the game, quits the client via alt+f4 and loads the game again? Or when he disables an addon, renames, loads, renames again and then enables the addon? Giving us the unique identifier is a good way to make sure that we have the right character, regardless of how often it is renamed, but using it as the key in the save data may be inconvenient for users that want to edit the save data from hand and putting the character name in the table is not really ideal as it could break an addon that doesn't expect the new entry in the table. Maybe you could extend the save data serialization so it can put comments into the file and add the character name there? That way it could be in the same line as the table key and would not interfere with existing addons. I am not sure how the API for that would look though. Maybe have a method to read and write comments for a key in a table? GetSaveDataComment(table, key), SetSaveDataComment(table, key, comment)? The case where a player disables an addon before the conversion to the new identifier, then renames and afterwards reactivates the addon would still remove the save data, but I think it could be neglected. I still believe having the account history would be a better way of solving this problem, but identifiers are also nice. Maybe we could also get them for another player's characters? Some addons save things about other chars and would not be able to follow renames, which would result in a loss of data.

I obviously missed the point of what to choose between....

/sign: Of course ID. I like the idea of comment as a hint.
And maintain a ID-to-Details table in ZO_Ingame:

["$Characters"] = {
   [ID] = {
      name = "abc",
      lastName = "xyz",
      lastClass = 1,
      lastGender = 2,
      history = { -- reserve for next release?
      }
      ...
   },
   [ID2] = {
   ...
   }
}

ZO_Ingame is the well-known variable you can save data in while pre-game.

[Justinon]

Originally Posted by ZOS_ChipHilseberg So here's what we were able to do for this issue: We added a function to get a unique identifier for the current character. It's a 64 bit number represented as a string. We have also added a new ZO_SavedVars constructor named NewCharacterIdSettings which will store the settings under the current character id instead of the name. It will also look to see if there is a setting table under the character name and automatically copy that to the character id key, then delete the character name copy. For most addons it should be sufficient to replace ZO_SavedVars:New with ZO_SavedVars:NewCharacterIdSettings to handle name changes. This has been done to all stock UI saved vars. Finally, we added an automatically generated key at the namespace level called "$LastCharacterName" which will hold the last name the character with that id had so the saved var files are a bit easier to read and modify manually. Let me know if there are any questions or concerns.

I think this is the best and safest way to go about resolving the issue. By doing it this way, it'll be easy to correct and then we won't have to worry about it anymore. Plus, I think it's more efficient this way.

[ZOS_ChipHilseberg]

We'll swap back to IDs then since it's more robust against names changing when addons are disabled.

[ZOS_ChipHilseberg]

So here's what finally went in:

1. The stock UI still uses character name. It will migrate settings tables between character names if the name changes. This has the least chance to break addons that are depending on the structure of the stock saved vars, is easiest to read, and is safe because the stock UI won't be disabled like an addon.
2. We re-added the character ID constructor in ZO_SavedVars named NewCharacterIdSettings. Addons can migrate to character ID settings for a solution that is robust against name changes when the addon is disabled. Character name based settings will be auto migrated to character ID keys the first time the addon is run using NewCharacterIdSettings.