👺 mobskill scripts audit needed

TeoTwawki commented 2 years ago

[x] I have paid attention to this example and will edit again if need be to not break the formatting, or I will be ignored
[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
[x] I have read and understood the Contributing Guide

Branch affected by issue

base

Steps to reproduce

Darkstar had some pretty janky mobskill scripts. Some may be so old as to actually be from PXI, but they've bene in every iteration of private server development.

The vast majority have calculations largely just made up with only ffxiclopedia as a guide and numbers modified by hand based on player memory from players who no longer had an active account.

But then besides that, some just have complete absurdities. Take the following example:

-----------------------------------
require("scripts/globals/mobskills")
require("scripts/globals/settings")
require("scripts/globals/status")
require("scripts/globals/msg")
-----------------------------------
local mobskill_object = {}

mobskill_object.onMobSkillCheck = function(target, mob, skill)
    return 0
end

mobskill_object.onMobWeaponSkill = function(target, mob, skill)
    local potency = skill:getParam()
    if (potency == 0) then
        potency = 12
    end
    potency = potency - math.random(0, potency/4)

    skill:setMsg(xi.msg.basic.SELF_HEAL)

    return xi.mobskills.mobHealMove(mob, mob:getMaxHP() * potency / 100)
end

return mobskill_object

The potency was intended to be set by the value of the skill param field in the mobskills sql table. If its zero, we default to 12. so far so good. But then we go and overwrite that with a random range from zero to one 4th of the original value. Instead of dividing it we could have just started with the lower number! But we're not done yet. When we get to the skill return we divide that again. Hey, I get it, the author wanted a float value to use with that max HP value. But we could have just defined the variable this way at the start instead of modifying in multiple steps, since we are doing no other math to this.

BUT WAIT, THERE'S MORE!

am positive this is not at all close to how the retail game determines how much HP pollen restores on the monster. In fact, I have reason to believe it is actually pretty similar to how the Blue Magic version of Pollen works. Most bee/wasp/hornet mobs seem to be war or thf. But a few are actually whm for either main or sub job. These whm ones have a significantly stronger pollen than their melee job counterparts. And when you look into how Blu's pollen is done you see it actually uses Healing magic skill.

Now I can't speak to how accurate we are in this, but we give mobs max skills (always capped) for whatever skills their jobs should have. Or we treat them as such (its not like mobs get skillups, we just tell it top use max). So I suspect if we did the calculation correctly, Demonic Tiphia's 3k ish pollen would fit right in on its own without our contributors doing work arounds they should not have had to do. They shouldn't be expected to try and fix this jank to add one mob and give it correct numbers.

And this is just one example. there are many like it.

Some data is available on the Japanese wiki, precious little elsewhere, and sources might not make a distinction between a mob version and a blue magic version for moves that exist as both (sometimes these would legit be the same, but often NOT, so we can't just take a wiki pages existence as proof that they are). By and large this will take massive testing sessions on retail.

Pollen info from JP wiki:

(MND x 3 + VIT + healing magic skill value ÷ 5 x 3) ÷ 2. * 4

I think that is just the cure magic formula for aoe cures. This should be easy enough to test if the math is accurate on a high level bee.

Expected behavior

Retail accurate maths in yer mobskill scripts skillin ur d00dz.

TeoTwawki commented 2 years ago

I think the 1st step here is probably to build a list of testable skills and a document to check them off as verified/correct or not as we go, like we used to do with quests.

Mortalelite commented 2 years ago

I do have all the mob skills typed out for anyone that needs, Added text file with all mobskills so we can start a list

Example ✔️ Check off when confirmed ❌ Unimplemented

✔️ 10000 needles ✔️ 1000 needles ✔️ 2000 needles

❌ doomvoid

Mobskills.txt

TeoTwawki commented 2 years ago

I was thinking something in repository. I made this starter file. In githubs markdown format in a gist so ppl can fork the document and change it as well. https://gist.github.com/TeoTwawki/a1d783f36ceebfa0ea34545e674fb868

Edit: List will have duplicates, that's ok. They can just both be greenchecked at same time.

Edit edit: I didn't see your attachment, same thing but in markdown with a table.

Mortalelite commented 2 years ago

I was thinking something in repository. I made this starter file. In githubs markdown format in a gist so ppl can fork the document and change it as well. https://gist.github.com/TeoTwawki/a1d783f36ceebfa0ea34545e674fb868

Edit: List will have duplicates, that's ok. They can just both be greenchecked at same time.

Edit edit: I didn't see your attachment, same thing but in markdown with a table.

Oh that works better than what I did setup.

LandSandBoat / server