DarkstarProject / darkstar

DEPRECATED - FFXI Server Emulator - See Project Topaz
https://github.com/project-topaz/topaz
GNU General Public License v3.0
455 stars 549 forks source link

Chasobringer kills while not on either quest not being counted #5791

Open chrisalleng opened 5 years ago

chrisalleng commented 5 years ago

I have:

Client Version (type /ver in game) : version 30181205_0

Source Branch (master/stable) : master

Found on Edenxi

Additional Information (Steps to reproduce/Expected behavior) :

Accepted "Blade of Darkness", equipped chaosbringer Killed 105 mobs, with only auto attacks Completed "Blade of Darkness" Character was not high enough fame to start "Blade of Death" Character killed another 100 mobs in the same way (total 205) Character accepts "Blade of Death" quest Character was not able to complete "Blade of Death" quest by trading the Chaosbringer in Gusgen Mines

poked around a bit, it doesn't look like ChaosbringerKills is incremented unless the player is on the "Blade of Death" or "Blade of Darkness" quests, which I do not believe is correct. Reference: https://ffxiclopedia.fandom.com/wiki/Blade_of_Death "You need to bring the total number of kills with the Chaosbringer (from the time you first received it) up to 200. As such, kills made before activating this quest count towards the total."

This also matches my memory, but it's been 15 years 😄

mobs.lua lines 16+ (having trouble with comment spacing, sorry):

function onMobDeathEx(mob, player, isKiller, isWeaponSkillKill)
    -- Things that happen only to the person who landed killing blow
    if isKiller then
        -- DRK quest - Blade Of Darkness
        if
            (player:getQuestStatus(BASTOK, BLADE_OF_DARKNESS) == QUEST_ACCEPTED or player:getQuestStatus(BASTOK, BLADE_OF_DEATH) == QUEST_ACCEPTED) and
            player:getEquipID(dsp.slot.MAIN) == 16607 and
            player:getVar("ChaosbringerKills") < 200 and
            not isWeaponSkillKill
        then
            player:addVar("ChaosbringerKills", 1)
        end
    end

EDIT: Also, it's an incredibly minor technicality, but to be the same as retail other abilities such as Jump shouldn't be counted - I'm not sure if isWeaponSkillKill includes other things like that, though. Frankly, the quest not counting abilities or WS kills was probably a bug in retail in the first place, not sure it's worth the effort of reproducing!

TeoTwawki commented 5 years ago

see https://guides.github.com/features/mastering-markdown/ scroll down to examples (its near the top) and click on code. this will make snippets of code properly readable.

we need isWeaponSkillKill replaced by a more generic set of info about what has happened to the mob so that we don't use half a dozen booleans like that on it instead, for now the only check we have is "was it a ws that killed this?". when we get to implementing magian trials this is going to matter: generic ws kills, specific ws kills, pet kills, magic kills...

TeoTwawki commented 5 years ago

we could fix the kill count of chaos bringer by checking if the 1st of the 2 quests is accepted OR completed prior to the 2nd quest being accepted, if thats how it works. I always thought it was saying that the kills during the 1st quest would still be counted when you did the 2nd quest though. (it was possible to throw the sword away, and then have to re obtain it and do twice as many kills..and you could go over during the 1st quest by a lot) /shrug

chrisalleng commented 5 years ago

Probably overstepping my knowledge of darkstar and why/how things are done, but I'm not sure you need to check the quest status at all - as far as I know, it's as simple as if a player kills something with an auto attack and has chaosbringer equipped as their weapon, increase counter. If it's dropped and has to be picked back up (from the zeruhn cutscene, which looks like it should work but I'm not crazy enough to try in a live environment) reset that counter back to 0.

They might have been storing the kills on the sword/item itself in retail or something equally crazy, which would explain why dropping and getting a new one would reset it...

There might be performance concerns or any large other number of reasons to check quest status, of course! I think you're right and checking if "Blade of Darkness" has been either accepted or completed should work just as well.

TeoTwawki commented 5 years ago

I just assume we don't want forever check if the player is wielding chaosbringer on every zone of every map server of every mob death for a quest line that may not have even been started (or has been finished already), for every player in existence. lets not increment vars before/after they matter Since you can't have the item (without cheating) without first having flagged the quests (and doesn't need to be counted after the 2nd quest completed), that seems like a good way to narrow it down.

retail prolly does something more like how ws points work on the item itself, except for..not ws. but we've no way to determine that since it wouldn't be apparent to the client, and this was less work to implement anyway.

teschnei commented 5 years ago

it would be about the same as checking if the player has started the quest on every zone of every map server of every mob death, they're both just a really quick lookup in an array

TeoTwawki commented 5 years ago

@teschnei see edit we'd still be doing more

ibm2431 commented 5 years ago

Regarding when checks are done, this line got me thinking:

when we get to implementing magian trials this is going to matter

And what I'll add: "Should RoE ever be implemented"

I wonder if we should start thinking about a way of doing "dynamic" checks for each player, so that for each player killing a mob we're not checking a large litany of specific equipment or quest activations.

This might be hard to convey without pseudo-code, but store the actual code checks as functions in a table (like how @takhlaq and I are doing on quest_rewrite), indexed by some master enumerated key-type (dsp.killcheck.CHAOSBRINGER, dsp.killcheck.MAGIAN666). Then for each player store as an array/table the current "killchecks" the server should be performing for that player (and update this array/table as players activate or finish quests). On mobdeath, grab that character's killchecks and just array-lookup and execute only the relevant checks for that player. That way we'd only be checking what's relevant for that player instead of every possible check on every kill.

This of course would be overkill just for this Chaosbringer scenario. Just something I wanted to murmur now to think about for the future..!

edit: In attempt to be clear, I'm not saying store the actual function for that player, just store the enum that can be used to look up that function. :sweat_smile:

ziaxe427 commented 5 years ago

im having a problem with this quest as well i HAVE the quest but and i swear i killed MORE than 100 but im not getting a cutscene for completeing the quest to unlock drk

TeoTwawki commented 5 years ago

im having a problem with this quest as well i HAVE the quest but and i swear i killed MORE than 100 but im not getting a cutscene for completeing the quest to unlock drk

that was caused by recent changes after the issue was already open, and fixed by PR that was just now merged :+1: