mtgjson / mtg-sealed-content

MIT License
5 stars 6 forks source link

change the type of variable.configs.chance #216

Closed kodawah closed 1 month ago

kodawah commented 1 month ago

We introduced the chance variable to accommodate for variable items appearing with a different drop rate (see https://github.com/mtgjson/mtg-sealed-content/issues/209)

Previously, the variable.configs could be marshaled to json as an array containing the list of possible items described as a key:value pair, where the value matched the data type of the parent level contents. This has a few upsides, namely being able to unmarshal the data as a map[string]content and make the code forward compatible with any new sealed type we might introduce.

The introduction of the chance variable broke this assumption and requires custom handling for each data type, and introduce a separate data type for each sealed product type. We also have the minor drawback that we don't see the total weight of the variable configuration, which makes us parse the whole variable configuration to understand the correct drop rate.

My proposal is: instead of introducing just a chance variable add a new sealed data type, variable_config containing chance and weight, for each element of the variable section. In other words

            "variable": [
              {
                "configs": [
                  {
                    "card": [
                      {
                        "foil": true,
                        "name": "Island",
                        "number": "551",
                        "set": "sld",
                        "uuid": "c536a9f4-bdc0-5bf1-828e-7cfd8e2c93af"
                      }
                    ],
                    "variable_config": [
                      {
                        "chance": 93,
                        "weight": 200
                      }
                    ]
                  },
                  {
                    "card": [
                      {
                        "foil": true,
                        "name": "Spellskite",
                        "number": "587",
                        "set": "sld",
                        "uuid": "632e35a3-81c9-5bed-aa6b-f1a77e5e1cff"
                      }
                    ],
                    "variable_config": [
                      {
                        "chance": 93,
                        "weight": 200
                      }
                    ]
                  },
                  {
                    "pack": [
                      {
                        "code": "blueprint-mk1",
                        "set": "sld"
                      }
                    ],
                    "variable_config": [
                      {
                        "chance": 14,
                        "weight": 200
                      }
                    ]
                  }
                ]

This makes the variable.configs data type consistent with itself, exposes weight (albeit in a redundant way, tho very human readable) and heavily simplifies the parsing of both variables.

Thoughts? @axxroytovu @ZeldaZach and @staghouse

kodawah commented 1 month ago

@axxroytovu can correct me if I'm wrong, but I think we cannot add chance and weight to the underlying content data type because it would break the configurations where you have multiple products in the same config, while in the new system it would be just a separate entry in the variable_config

axxroytovu commented 1 month ago

How are you handling card_count which is currently an integer?

I'm working through how to make this work, but distributing the weight correctly into nested variable maps is proving tricky.

kodawah commented 1 month ago

I don't use card_count but from what i can tell, it gets exposed as a plain field outside of contents no?

axxroytovu commented 1 month ago

Ah, it gets pre-processed by MTGJson.

kodawah commented 1 month ago

that could be another option, having an array of variable config outside of contents to be read only in case the sealed product contains variable stuff