Improve upon Setting document cast and onChange behavior to ensure that each Setting document is cast to its configured data type as part of the initialization workflow. This allows the cast value to be always past to onChange handler instead of the raw string.

[mclemente]

What happened?

I have a 3-choice setting that logically is handled as a Number, but uses the choices value instead of a range to make it easier on the user to see what choice entails.

Example:

{
  ...settingData,
  type: Number,
  default: 0,
  choices: {
    0: "All",
    1: "PCs",
    2: "NPCs",
  },
  onChange: (value) => { game.MyModuleName.settingName = value }
}

When the value is changed, the value is sent as a String, but when calling game.settings.get("MyModuleName", "settingName"), it returns a Number as expected.

Due to this, if it is set to 0, the variable is set to "0" instead and breaks logic like if (game.MyModuleName.settingName).

game.MyModuleName.settingName is setup to avoid making multiple game.settings.get calls during play for a setting that won't be changed often.

What ways of accessing Foundry can you encounter this issue in?

• Native App (Electron)
• Chrome
• Firefox
• Safari
• Other

Reproduction Steps

1. Make a setting similar to the example provided above.
2. Add a breakpoint on the onChange.
3. Change its value.
4. Check the value set to game.MyModuleName.settingName.

What core version are you reporting this for?

V11P1

Relevant log output

No response

Bug Checklist

• The issue occurs while all Modules are disabled

[aaclayton]

I'm not sure I understand what is the "bug" here. The way you have configured this setting, it will store one of 3 things into the database 0, 1, or 2. If the value of your setting is 0, you are always going to need some different logic in your module to deal with logical conditions that differentiate between "the setting has not been set" vs "the value is zero".

Shouldn't your module code just be:

if ( game.MyModuleName.settingName !== undefined )

[mclemente]

The bug is: "Setting._onUpdate shouldn't be sending its value property as a different type."

The bug is that the value sent during Setting._onUpdate call is a String instead of a Number, and that only happens if setting has choices set.
As in, if you change a Number-setting that has a range, the document's value that is sent as a parameter to the onChange is a Number.

Doing the same for a Number-setting that has choices, the value sent is a String.

Looking further, at ClientDatabaseBackend._postUpdateDocumentCallbacks, a range sends a value: "1", while a choice sends a value: "\"1\"".

---

The if-statement was meant as a zero/non-zero check, not to check if it exists/isn't undefined. If the setting is either 1 or 2, it executes.

This could easily be fixed by adding a "!= 0" or something similar, yes, but the issues lies in how that can lead to some weird behavior if you're not aware of that.

setting = {
  type: Number,
  default: 0,
  choices: { 0: "Hide stuff", 1: "Show stuff", 2: "Show to GM only" },
  onChange: (value) => game.MyModuleName.settingName = value; // Number(value) would fix this
}

game.MyModuleName.settingName = game.setting.get(setting) // 0
if (game.MyModuleName.settingName) //won't execute, correct
if (game.MyModuleName.settingName != 0) //won't execute, correct
if (game.MyModuleName.settingName !== 0) //won't execute, correct

game.setting.set(setting, 1) // setting = 1, game.MyModuleName.settingName = "1"
game.setting.set(setting, 0) //setting = 0, game.MyModuleName.settingName = "0"

if (game.MyModuleName.settingName) //executes, incorrect
if (game.MyModuleName.settingName != 0) //won't execute, correct
if (game.MyModuleName.settingName !== 0) //executes, incorrect

[aaclayton]

the reason the data is a string in Setting._onUpdate is that all settings are saved as strings in the database rather than mixed types. The _onUpdate is related to the document update workflow.

I do agree it's a bug that the setting onChange callback is providing the wrong data type though, I think we can prioritize a fix for that.