Digus / StardewValleyMods

My Stardew Valley Mods
GNU General Public License v3.0
17 stars 29 forks source link

Suggestion: [PFM] Make "Probability" in Star Quality Inputs default to 1 instead of 0. #210

Open HaulinOats opened 2 years ago

HaulinOats commented 2 years ago

I was a little confused when building a PFM mod that the "Probability" field in Star Quality Input objects defaults to 0 while being optional. I feel like it makes more sense to make it default to 1 unless people specifically set it to another value mostly because if you've built all the objects already for having varying star quality inputs, it doesn't make sense that they still wouldn't do anything until you actually set the optional "Probability" field as well. Defaulting to 1, unless a different value is placed, seems to make much more sense to me.

Anyways, just a suggestion since it caused me some confusion, but perhaps you have bigger reasons for this, so no worries if that's the case.

Digus commented 2 years ago

There is a reason. If set to 0, If all other probability fields do not add up to more than 1, it still has a chance of being "draw". So if you have 3 possible outputs and they are all 0, it they have a chance of .33 each. If you have 3 possible outputs, one with 0.5, the other with 0, the other will have 0.25 each. So zero is your default value, when you don't care to set a fixed probability, it evens out between all the possible ones. 1 is better used when you have a condition choosing your output, so if that condition is matched, it will certainly be the one draw. Order is also important, since outputs are draw in order.

HaulinOats commented 2 years ago

I see. I had something like this going on:

 {
    "ProducerName": "Fish-2-Seed Processor",
    "InputIdentifier": "682",
    "InputStack": 1,
    "MinutesUntilReady": 20,
    "OutputStack": 15,
    "OutputIdentifier": "Corn Seeds",
    "Sounds": ["Ship"],
    "SilverQualityInput": {
      "OutputStack": 25
    },
    "GoldQualityInput": {
      "OutputStack": 35
    },
    "IridiumQualityInput": {
      "OutputStack": 50
    }
  }

And felt like that should have worked but had to do this instead:

  {
    "ProducerName": "Fish-2-Seed Processor",
    "InputIdentifier": "682",
    "InputStack": 1,
    "MinutesUntilReady": 20,
    "OutputStack": 15,
    "OutputIdentifier": "Corn Seeds",
    "Sounds": ["Ship"],
    "SilverQualityInput": {
      "Probability": 1,
      "OutputStack": 25
    },
    "GoldQualityInput": {
      "Probability": 1,
      "OutputStack": 35
    },
    "IridiumQualityInput": {
      "Probability": 1,
      "OutputStack": 50
    }
  }

It just threw me off with "Probability" being optional but doing nothing if an "OutputStack" value exists, but just felt like making my case. I do appreciate PFM quite a lot and the hard work you've clearly put into it. Hope my feedback doesn't come off as rude, just felt like perhaps mentioning it in case someone else possibly had the same issue, or rather, assumed just putting an "OutputStack" as the singular object value would make it behave as if the default probability was 1.

Digus commented 2 years ago

You didn't come of as rude at all. And actually I was wrong. I answered you thinking of output probability.

Probability on the Stack Config default to zero and make the config useless if a probability is not set. But I'm afraid changing it now might change some mods behavior. I will think about it.

HaulinOats commented 2 years ago

Right, I know you can't make such changes without breaking compatibility, just wanted to make my case. Might be worth updating the documentation to be a little more clear that that will be the default behavior if "OutputStack" is the only field on the object, but again, I also could have read a little more closely. No worries :-)