Move skillratio to skill_db.conf

[guilherme-gm]

I think this issue may be considered as a small part of #73 , but I thought it would be better to make it as a separate issue so we can tackle this issue isolated from the rest.

I am up for working on this. I just want to grab some feedback on the approach before I go ahead.

Is your feature request related to a problem? Please describe.
Today we have this huge battle_calc_skillratio function with 2 levels on switches for pretty much every skill in the game. Currently it takes around 1500 lines with formulas for each skill and some fall throughs.

To add to the complexity, it has a mix of sometimes adding to the received paramter "skillratio" and other times replacing the value. And well, skillratio is always 100 when this function gets called.

In my opinion, this huge function block with formulas + the skillratio parameter makes things quite confusing. For example, there we many times when changing a skill where I had to wonder whether 100% was forgotten in the formula, or whether I have to reduce 100% when comparing to the description, etc.

Describe the solution you'd like
My proposal is to rework this part so:

1. "constant" values are moved to skill_db.conf;
2. Values that still needs to be calculated are moved to separate functions;
3. We don't have to think about this incoming skillratio = 100 parameter

This would remove those hardcoded values and should make tweaking basic skill settings easier and pluggable. In the other hand, skill_db.conf will grow a few thousand lines.

A very high-level draft of what it would look like may be found here: guilherme-gm@265fe57

Scrolling through this function, I can see a few things that jumps to the eyes:

1. Cases where we have simple, "constant", formulas (e.g. skillratio = skill_lv * 10 + 30;)
2. Cases where we have simple, "constant", formulas with a small condition (e.g. extra boost from a SC)
3. Cases where we have simple, "constant", formulas with extra RE level boost (i.e. simply call RE_LVL_DMOD after the formula)
4. Cases with 1 condition and 2 simple "constant" formulas (e.g. Bows causing half damage vs other weapon)
5. Cases with several conditions, logic, damage differences, etc
6. There are only 3 skills today that uses both Magical and Weapon formulas (the external switch), but as far as I can tell, we can easily work around them using SkillData fields. Those are the skills:
   • LG_SHIELDSPELL (Lv1 is WEAPON, Lv2 is Magical)
   • LG_RAYOFGENESIS (Does both magic and physical damage at the same time)
   • SO_VARETYR_SPEAR (Does both magic and physical damage at the same time)

My initial proposal is the following additions to skill_db.conf:

	SkillInfo: {
		// This will tell us that we need to perform a custom coded formula
		// While we could simply do a lookup whether we have a function, I think having
		// a field here makes clearer to people reading the DB.
		// like "Oh, ok, there is a ratio here, but I should expect more things on code"
		// Also allows us to detect missing formula functions during startup.
		CustomSkillRatio: true/false (boolean, defaults to false)
	}
	SkillRatio: {
		BaseLevelModifier: 0 (RE-only? Applies the RE_LVL_DMOD value)
		Lv1: 100
		Lv2: 100
		...
	}

And inspired on gepard-me suggestion in #73, we would have a function that populates a db of SkillID -> SkillRatio function for the skills that needs a formula.

---

Going back to the cases I mentioned above, we would have:

1. For simple constant formulas -- Simply expand the formula into SkillRatio values
2. For extra boosts from SC -- Create a function for CustomSkillRatio and use SkillRatio + function (we could use SkillDataN fields to help too, or some sort of new field if we want to make it generic)
3. For RE boost -- Simply expand the formula into SkillRatio + set BaseLevelModifier
4. For cases like bow does less damage -- Create a custom function and use SkillRatio + SkillDataN
5. Cases where everything is crazy -- probably just hardcode the entire thing in a function

We may expect to get a good amount of new, small functions, but I think this is fine(?)

For people with customization
For people with many customizations, this should not have huge impact, they could migrate their changes by simply making a "get all" function with their current switch/cases and make their skills a "CustomRatio" skill. Some manual work, but not a huge effort, I think.

It would look like that:

int skill_my_custom_ratios(... all parameters here ...) {
  int skillratio = 100; // current code gives skillratio = 100
  // original code
  switch(skill_id) {
     case My_Custom_Skill1:
          // original code
  }
  // ...
  return skillratio;
}

in the db setup:

idb_put(skill_ratio_db, My_Custom_Skill1, skil_my_custom_ratio);
// repeat for each custom skill

and adding this to every customized skill:

SkillInfo: {
   CustomSkillRatio: true
}

Describe alternatives you've considered
I have also considered tweaking a bit SkillRatio from the example above so instead of Lv1..Lv10 you could specify a "step". Since many formulas are like "start at 100, add 10 for each skill level". So we would have something like:

	SkillInfo: {
		CustomSkillRatio: true/false (boolean, defaults to false)
	}
	SkillRatio: {
		BaseLevelModifier: 0 (RE-only? Applies the RE_LVL_DMOD value)
		BaseRatio: 100
		RatioPerLevel: 10
		...
	}

Additional context
I think there are other aspects of skills that could follow similar approach, but I think it is better to take it slow so we can actually do it.

---

As I mentioned in the start, I can work on this change, but I want to get some feedback on the idea before I start coding it.

[TioAkima]

I liked your proposal.
Bringing the skillratio to the skill data base is a great idea, it makes the entire structure more organized.
My server has many customized skills, so having this facility in manipulating the skillratio through skill_db is interesting, very intuitive indeed.

Simple formulas (that do not require a special formula) simply vary the % by the skill level.
For those who want to change the default balancing, it seems very practical to me.

It wouldn't be possible to include formulas with attributes, right? In this case we would use CustomSkillRatio for more elaborate calculations.

(BaseRatio and RatioPerLevel is also excellent*)

[guilherme-gm]

Thanks for the feedback.

About attributes (I believe you meant stats like STR/AGI/etc), you are right, it would need the use of custom formula.

Since we are not doing formula evaluation in db, I think moving stats as having their own fields would bring too much clutter to the skill db, and usually they are not very straightforward in formulas. Some gives % to ratios, some needs like 10 points for 1% and so on. Doing that via db seems like too much. We would need to make too many names and conditionals that would make it hard to maintain.

In the other hand, one could make a custom ratio formula that uses skill data somehow for that. For example, a custom ratio that does SkillRatio + STR * SkillData1. I made an example in the main post (check the repo link) where I did something like that for ninja charms instead of str.

Edit: link to charms example - guilherme-gm@265fe57#diff-759fea9dcd40522e86217dd352aadd687488dff49c67aa880610d4aeaaeb89c0R955

[TioAkima]

oh yes, the idea already includes being able to create personalized formulas (using str, agi, among other conditions), so it's great.

well... ease of manipulating the skillratio with improved organization and customization support.
I can't think of an example where it wouldn't work.
great job.

[csnv]

I also think is a nice idea and a first step towards having a one skill-one function structure. It also helps removing skill related stuff from battle.c.

The second alternative for the conf structure seems a bit alien. The first one goes better with what we already have, even if it's a little bit more verbose.

On another note, skill.c is already 25k lines long. Maybe it's better to add another file for this purpose? Something like skillratio.c, as it would also host quite a few custom functions. Maybe we should also split skill_db.conf into separate files.

[guilherme-gm]

Yeah, I like your points. Totally agree with new files too.

I am not sure about how we could split skill_db.conf, the cleaner way seems to be by category like jobs/npc/homun/etc. but there are gaps in the middle, which may make it harder to find gaps where skill ids are empty.

For example, after DC_SERVICEFORYOU, there are 3 NPC skills, followed by 3 marriage skills, 1 item skill, and back to a few NPC skills.

If we wanted to keep the ID order, we would need 4 files here (Npc1, Marriage1, Item1, Npc2).

Another option would be to split by ID range, but in this case we would have every file with a mix of types of skills

Yet another option would be to simply group by NPC / Jobs / Marriage / etc, but in this case we would have something like that: (All ids are made up)
Job.conf : IDs 1 .. 100 , 150 .. 200, 1000 .. 1100
Npc.conf : IDs 101..110 , 400 .. 600, 1300 .. 1500

And it would be harder to, for example, tell that there is a gap between 110 and 150. One could still go to source and check the enum, though (which seem easier than reading the db anyway)

[csnv]

I may have spoken too fast about splitting skill_db.conf. Seems like, contrary to source files, configuration files that share the same settings/properties, have no problem being too large. It's plain text after all.

It does break the blame tool of github though, that's the only disadvantage I see.

[guilherme-gm]

I do like the idea of splitting if we have a good approach xD although it works in a single file, too big text files is still hard to deal with

[SombraRO]

I remember that they once posted something similar on the Hercules forum but it had other settings besides the skillratio, I think it would be very interesting if this were applied and not just limited to the skillratio. It would greatly avoid using the compiler to make changes to skills

Here is the link, the person who wrote about it was Beret

https://board.herc.ws/topic/15716-new-fields-in-skill-db/?tab=comments#comment-86650

[guilherme-gm]

I remember that they once posted something similar on the Hercules forum but it had other settings besides the skillratio, I think it would be very interesting if this were applied and not just limited to the skillratio. It would greatly avoid using the compiler to make changes to skills

Here is the link, the person who wrote about it was Beret

https://board.herc.ws/topic/15716-new-fields-in-skill-db/?tab=comments#comment-86650

I lke this and agree. But I would rather take one step at time than do everything at once