🐛 mobskill scritps not using mobs weapon damage properly

[TeoTwawki]

I affirm:

• [x] I understand that if I do not agree to the following points by completing the checkboxes my issue will be ignored.
• [x] I have read and understood the Contributing Guide and the Code of Conduct.
• [x] I have searched existing issues to see if the issue has already been opened, and I have checked the commit log to see if the issue has been resolved since my server was last updated.

OS / platform the server is running (if known)

n/a

Branch affected by issue

base

Steps to reproduce

We've a bunch of scripts that never include the mobs weapon damage at all. This is the primary way these skills will scale damage for the mobs level: a lv 7 rock lizard and a lv 40 lizard in yhoator aren't going to hit the same.

Presently the skills correct look like:

local dmgmod = 2.5
local info = xi.mobskills.mobMagicalMove(mob, target, skill, mob:getWeaponDmg() * dmgmod,

while the ones incorrect look like:

local dmgmod = 6
local info = xi.mobskills.mobMagicalMove(mob, target, skill, dmgmod,

With that 4th parameter being the mobs total weapon damage to be used in the calculation for damage. In the second example its a static weapon damage of 6, instead of getting the monster real weapon damage value and then multiplying to whatever degree is needed (multipliers can be less than 1.0 to reduce as well)

However, we should not continue this pattern (see below)

Expected behavior

Instead of adjusting every script missing it to call mob:getWeaponDmg() we should bake that into mobMagicalMove(), mobPhysicalMove() etc. And then mass remove it from existing script so that all that remains in the multiplier itself. For skills not using multipliers it would just be 1. This eliminates this type of contributor error from happening again.

Unfortunately this also means a ton of mobskills will need their actual damage checked in game, since a bunch of them are wrong having been tuned by someone's eyeballs under the specific conditions they were testing with, while making this error. (Related: none of these are tuned with math, its all eyeballing)

see also: https://github.com/LandSandBoat/server/pull/4912#discussion_r1437693682

[Frankie-hz]

Jimmayus had done a full COP monster ftp retail data for us here: https://docs.google.com/spreadsheets/u/0/d/1YBoveP-weMdidrirY-vPDzHyxbEI2ryECINlfCnFkLI/htmlview#gid=0

If this is ok to reference as a source this is one of the things I need to clean up and port for our asb rebase.

Things to note: Ftps are mostly derived from Monstrosity and BST monster ftp with existing monster data.

FTPs are approximate and further retail testing will be needed probably for the next years to come

As of right now there is no known scaling of FTP but when we revisit for toau+ there will potentially be. As a safe measure I want to remove the use of “dmgmod” and properly provide ftp0-3000 for future cases.

If this is something we can agree on and use then I can start this sometime in the next few weeks little by little.

[TeoTwawki]

Jimmayus had done a full COP monster ftp retail data for us here: https://docs.google.com/spreadsheets/u/0/d/1YBoveP-weMdidrirY-vPDzHyxbEI2ryECINlfCnFkLI/htmlview#gid=0

If this is ok to reference as a source this is one of the things I need to clean up and port for our asb rebase.

Things to note: Ftps are mostly derived from Monstrosity and BST monster ftp with existing monster data.

FTPs are approximate and further retail testing will be needed probably for the next years to come

As of right now there is no known scaling of FTP but when we revisit for toau+ there will potentially be. As a safe measure I want to remove the use of “dmgmod” and properly provide ftp0-3000 for future cases.

If this is something we can agree on and use then I can start this sometime in the next few weeks little by little.

considering what exists now is largely made up bs repeatedly copy pasted and then massaged till it "looked like what someone saw on retail this one time" with no actual attempt to predict what values would equal what damage on retail and compare, I don't see a problem with using that source.

I do think actual getting of the mobs wdmg should not be in any individual scripts tho, so anywhere we need to "get" should be handled in the global as mentioned in my suggested solution. Issue was originally opened more so for that and the resulting mistakes caused by 2 diff styles, rather than the fact all our calculations have been bs since before dsp even opened its repo. I'd pretty much given up hope that they'd be done correctly in my lifetime.

[claywar]

Heads up on this train of thought as well. To echo Teo, there's a lot of copy-pasta and inconsistencies in those scripts, and I'd like to find a more common and clear way to represent most skills. Open to ideas.

[Frankie-hz]

I was thinking we structure it like xaver did magic and future of weaponskills. One global with a table of all skill names, ftp, etc

There are a lot of skills that do very specific things with dInt, mob base damage, additional dstat, etc. but I think the global tables you guys have been making is the best way to go.

[TeoTwawki]

1 difference on that, with spells we basically wind up listing off every spell for a lot of it. there are far more mobskills than spells, I wouldn't want to do that - to big and unwieldy, border son unmaintainable. but reusable functionality we pass parameters to, that we can do. Instead of a column holding which dstat to use, we instead just along which one this skill uses, and do all the work with that in the global (don't "get" the stat at the mob, but just pass on "we're using INT here") instead of all those in vs int checks in the skill script. I would also like to see more common ground between weaponskills and mobskills, particularly since there are mob versions of almost all the weaponskills and a bunch of them basically ARE weaponskills but we've been faking it.

this may have been obvious but i figure better to head off any attempt at a gigantic enumeration of every skill name to have all its data after it in the single largest lua table ever conceived of.

[Frankie-hz]

Ah you right, I did not think about how big of a file that would be. Would this be better started then after the weaponskill rework is done? I can keep our stuff modularized for now until that is finalized so we have a better idea of how we want to format it.

[TeoTwawki]

Would this be better started then after the weaponskill rework is done?

For major structure changes probably so. That doesn't mean we can't greatly clean up mobskill scripts in the meantime, and make some things make sense while doing some function argument cleanup.

[TeoTwawki]

update: https://github.com/LandSandBoat/server/pull/5062 baked in the multiplier and I didn't even realize that happened since the issue wasn't linked, so now all we need is an audit of the skill scripts.

edit: I'm told it was actually another pr before that, but git blamed 5062 for it /shrug