Wow, you've given me a lot to reply to, but that's good, I really appreciate the feedback. I cut some of your post together to cut down on the amount of spaghetti in my response.
A. Base value
===========
Percentages need a base value as to what constitutes as 100%, there are multiple candidates one can chose:
1) Max of soldier 'statCaps'
2) Max of soldier 'trainingStatCaps'
3) Max of current soldiers on craft
4) Max of current soldiers on base
5) Max of all soldiers still alive on the globe (e.g. all bases)
6) Something else ...
Based on your post I suspect you've chosen '1' but it would not hurt to specify which one (or multiple?) you want to support.
A. Option (1) seemed to be the most logical one. Certainly more stable than 3-5 (not depending on RNG and not varying during the course of a campaign).
Should prevent situations where: "One-eye would become *the* marksman in the land of the blind".
I'm working on a design that is flexible and allows (potentially) multiple designs. (Specifically, I'm reworking how the `StatStringCondition` class works so that it has a bunch of sub-classes that can implement different behavior. Its `isMet` method will now take a reference to the soldier, to allow a lot of flexibility in this). Under this design 1 and 2 are trivial, and 3 and 4 might be possible (soldiers have a reference to their craft and craft's have a reference to their base). Though I don't really see working on 3 and 4 unless there is a lot of desire for it. 5 is likely more difficult as there doesn't appear to be an easy way to get a reference to all soldiers from here (a problem I've run into elsewhere).
In any case, I plan on (well this part is already done actually) #1 as the primary function. I also added support for number #2 while typing this response... I might back it out though. I think there are better ways to display this information that I might address in a different patch.
B. Stat overflows
=============
It is possible for soldiers to slightly overflow a stat (e.g. percentages > 100), at least for the fixed base values.
Will you cap the values at 100 or recognize percentages above 100?
B. I believe this is less of an issue since I suspect most users will opt to use something akin to '[90, ~, p]' instead of '[90, 100, p]' but it is something to consider.
My (weak) preference would be allowing percentages > 100. So one could designate: "the best of the best of the best ..." (e.g. '[100, ~, p]').
The easier the logic, the better. So I'll copy the existing functionality here. If you feed it "~" (or actually any non-integer) as the first value in the array, the minimum value will get set to 0 (or 0%). If you set "~" (or any non-integer) as the secondary value, the maximum value will get set to 255 (corresponding to 255%). I don't see any specific reason to limit the ranges otherwise (and the existing code doesn't do so) so yes, percentages over 100% will be allowed. Stats less than 0 aren't meaningful in X-Com, but who knows what might happen in the future, so I'll allow for negative percents as well (the current code allows for them FWIW). We'll assume the user knows best what they are doing if they set some funky values or edge-cases.
C. Behavior for base values != 100
===================================
From experience most (max) stat values tend to not align neatly with 100, meaning any percentage based range will lead to gaps or duplicates.
For reference, given the following statstring definition:
statStrings:
- string: "x"
firing: [~, 50, p]
- string: "M"
firing: [51, ~, p]
If the base value is < 100 one could end up with:
"SoldierName/xM"If the base value is > 100 one could end up with the following naming scheme when a soldier levels up:
"SoldierName/x" -> "SoldierName" -> "SoldierName/M" (e.g. missing statstring when a number falls between 50% and 51%).
How would you tackle this?
C. Never found a good way to solve this (yet), currently I tend toward the solution that does extra calculations:
- If value is at upper boundary (e.g. '[~, 50, p]') calculate the next one up (e.g. '[~, 51, p]').
+ Use the midpoint for a decision if base value > 100.
+ Use floating point rounding for a decission if base value < 100.
- If value is at lower boundary (e.g. '[51, ~, p]') calculate the next one down (e.g. '[50, ~, p]').
+ Use the midpoint for a decision if base value > 100.
+ Use floating point rounding for a decision if base value < 100.
Wow, you've put a lot of thought into it. My current code here is very basic:
const int stat = (*soldier->getCurrentStats())[_conditionName];
const int cap = soldier->getRules()->getStatCaps()[_conditionName];
// percents are stored as integers, ie 50% = 50, so multiply stat by 100 to offset.
int percent = (stat * 100) / cap;
return percent >= _minVal && percent <= _maxVal;
(not tested yet either). I'll investigate more deeply when I get a bit further in design. I was planning on treating the percents as integers in definition, but if necessary switching to floats is possible.
D. Allow both fixed and percentage for the same stat.
========================================
Although you've already touched this subject it is a bit unclear if you want support "one OR the other" or "both at the same time".
E.g. is the following example supported?
statStrings:
- string: "x"
firing: [75, ~, p] # Percentage based but it could map to a value of [40, ~] if base value is pretty low.
firing: [60, ~] # We only consider soldiers that can identify: "the business end of a gun".
D. Due to the nature of some stats a fixed minimum might be desired, hence my preference to support both at the same time.
Again: "one-eye ... land of the blind".
Yes, under my current plan, this is supported. Every line corresponds to a new condition, so you can have percentage-based conditions and traditional conditions within the same rule.
E. Backwards compatibility.
=====================
When using any new definition one probably want to prevent errors (or undefined behavior) when a user tries to load it into an older OXC(E) version.
Worst case the application could choke (crash) on the ruleset. More likely it could lead to unexpected designations (percentages being treated as fixed values).
Any idea how to approach this?
E. Not a strong one since 'metadata.yml' value "requiredExtendedVersion" could prevent this situation, something to consider though.
Well, there is nothing I can do about a new ruleset being used on an old version. Since I can't change old code, incorrect behavior is inevitable. However, under my current plan, it would not crash (elements of the array beyond the first two are just ignored). But as you say, the percentages will be treated as fixed values. If I instead treat the percentages as floats, then I think the most likely behavior is they will fail the conversion, and the minimum (0) and maximum (255) values will get used instead.
Honestly, though this isn't a thing I worry too much about. The new rules won't cause things to blow up if run with an outdated version which is about as good as can be asked for. However, your sidenote offers other possibilities.
Sidenote (since you are open to suggestions).
+++++++++++++++++++++++++++++++++++
Instead of adding an option to the range array, why not use specialized value definitions (like for example 'firingPercentage: [~, 66]')?
In my opinion that would make the desired use case stands out a bit more in the ruleset.
Wow, this is a great idea, I'm upset I didn't think about it. It makes my design slightly more complicated in someways, but simpler in others (hard to explain without posting all the code). The biggest advantage is backward compatibility would be 0 concern. My current read of the code is that condition names not present in the preset list would simply be ignored. I'll strongly ponder this design.
As for the statstring divider
++++++++++++++++++++
Although I do not deem it that important, percentage based statsrings deliver more QoL, I do believe it could be beneficial to personalize OXC(E) behavior.
However, there are as many opinions for separators as there are users (for example, "/", "\", " ", "|", etc...).
This means a translatable string (e.g. 'STR_STATSTRING_SEPERATOR' or something akin) has the best potential to cater for user wishes.
Since i understand bringing 'state' into 'soldier.cpp' might be hard (or nearly impossible) you might want to consider the following alternatives:
+ Add a new statstring top-level attribute e.g (replacing the current hard-coded one).
statStrings:
- divider: "|"
- string: "x"
firing: [75, ~, p] # Percentage based but it could map to a value of [40, ~] if base value is pretty low.
firing: [60, ~] # We only consider soldiers that can identify: "the business end of a gun".
+ Add a divider before each individual statstring (so one can end up with something akin to "Soldier|9|6|8|1|5")
statStrings:
- string: "x"
divider: "|"
firing: [75, ~, p] # Percentage based but it could map to a value of [40, ~] if base value is pretty low.
firing: [60, ~] # We only consider soldiers that can identify: "the business end of a gun".
I had considered this as an option. The problem with it is it means either storing the statString divider in the `Soldier` class, where it seems a bit out of place or diving somewhat deeply into the statString class to pull this bit of data out. And the problem here is that there is not just one instance of the `StatString` class, but a vector of them, one for every "rule." Neither are necessarily show-stoppers, but the design feels inelegant. I'll ponder.
The problem with your second option is there simply is not enough screen real estate to make this version very useful. Every character is at a premium.
Hope this post was of any use.
It was
incredibly helpful, thanks so much for the feedback.
Show Posts
