From what I have glimpsed in the code and on the wiki, on two types of bonuses can currently scale with level, and the code for this, in CGHeroInstance::Updatespecialty() is rather ugly. I would propose the following redesign for improved flexibility:
In HeroSpecial, replace field growsWithLevel with two new floating-point fields valBase and valPerLevel.
In CGHeroInstance::Updatespecialty() field val is then recalculated as val = floor(valBase + level * valPerLevel).
Legacy json is converted into new structure when parsed.
Bonuses that don’t grow with level can be expressed by leaving valPerLevel = 0.
Issue that I can see with this:
It won’t be possible to express original HMM3 bonus progression for creature specialty exactly, due to intermediate rounding.
Need to be careful with rounding to ensure e.g. that valPerLevel = 0.1 evaluates to 1 at level 10, not as floor(10*0.099999999) = 0.
Well, 100% compatibility with base game mechnaics is a priority. I agree, though, that the currnet solution is ugly.
Apart from that, I’ve had an idea to add special modifiers to bonus itself (as another field, similiar to Limiter or Propagator), which evaluates it based on any other factor (such as hero level).
Another issue to consider is to convert “specialties” field in config/heroes/ to use proper Bonus format.
Just curious, can we replicate original formula using only integers? Use int64_t for intermediate values and scale inputs and outputs (f.e <<32 input and >>32 result)
Alright, I suppose the cleanest solution would then be to use three variables instead of just two: valBase, valPerStep and stepSize, which then combine as
which would enable us to express original bonuses precisely by setting valBase = 0, stepSize = creature_level and valPerStep = [attack|defense] * 0.05. Probably more complex than typically needed, but with proper default values (valBase=0, valPerStep=0, stepSize=1) it shouldn’t be an issue.
Good point on the legacy “specialties” field - the wiki already suggest replacing this, but not sure whether this has been implemented yet.
If we want to do it with integers, I’d suggest a base of 20 (which is used by HMM3 skills), giving us
val = valBase + ceil( valPer20Steps * floor(level / stepSize) / 20 )
which eliminates the potential issue of rounding errors (that are present in current code), and simplifies the settings for HMM3 creature specialty to valPer20Steps = [attack|defense].
As we’ll still be parsing deprecated bonus style and converting to new one, I figure that may as well be implemented within vcmi, e.g. as log info when encountering old style - something like
encountered deprecated specialty format for [hero] ([faction]), please convert to [new-style-json]
which would also help with updating mods still using old style (if there are any).
Less coding work than python script, but more copy-paste work. Though I thought I saw @misiokles put his hand up here?
Hero json format was first published with new speciality format in December 2012. Just refuse to load mods with old format. I`m sure there is no such mods.
UPD. but new format itself has been changed few times.
Now, I’ve come across an issue where VCMI creature specialty bonus might(?) differ minimally from HMM3. The issue is that I suspect (anyone knows for certain?) that in HMM3 the attack/defense bonus depends on the creature’s attack/defense (you’ll notive that archers have 6 attack and 3 defense). If so, then some upgraded creatures may be short-changed because they only get the bonuses of the base creature. No difference for archer vs. marksmen, but e.g. griffins have 8/8 while royal griffins have 9/9.
It’s not very hard to fix this with the new system - just use separate bonuses for base and upgraded creatures where needed. The only question is whether it’s worth having (up to) 5 bonuses for creature specialties instead of the current 3 - the hero json are getting bulky enough as it is. Thoughts?
Current implementation is somewhat confusing. While creature bonuses can be readable, secondary skill bonuses using ScalingUpdater get value of 100. And that means 5% for level, which is far from intuitive. A easy solution (for users) would be to have separate updaters for creature stats and secondary skills / spells / Adela’s bless that actually reflect their value.
Also, is this expandable for other updaters? An obvious example could be updater that scales with stack level (like WoG Darkness Dragons and Peasants generating gold). More common ideas:
Scaling commander abilities with commander level
Scaling hero bonus with commander level (common in WoG)
Artifact or spell bonus that still scales with hero or commander level
How complicated would it be to implement such cross-node dependencies?
I don’t feel that specifying values per 20 levels (or steps) is that counter-intuitive, but that may be because I’ve worked with it too much. Adding some extra (redundant but simpler/more readable) updaters is certainly doable.
The system is certainly expandable - anything that inherits a bonus can modify it on the way (e.g. hero can modify bonus provided by artifact before passing it on to creatures controlled). So …
Artifacts can already have bonuses that scale with hero level.
For spells this doesn’t work, as spell effects are directly attached to creatures without passing through hero.
Adding an updater that updates bonuses at stacks based on level of controlling hero will be easy. Generalizing this to updaters that consider arbitrary ancestors/descendants would require more though (i.e., not that easy).
Updaters that scale with stack level will be easy to add as well.
I haven’t really looked at commanders yet, but I imagine that creating an updater that scales bonuses based on commander level should be easy as well.
Well, it cannot stay like that. There is a given formula to automatically calculate the bonus based on creature stats and level, moving it to new format gives both incorrect values and more complicated text…
