project-topaz / topaz

Server emulator for FFXI
Other
158 stars 223 forks source link

Mining breaks Sneak #1174

Open Era-Lusiphur opened 3 years ago

Era-Lusiphur commented 3 years ago

I have:

Additional Information (Steps to reproduce/Expected behavior) :

It appears as if the end of the trade process, all detectable effects are removed from the character: https://github.com/project-topaz/topaz/blob/release/src/map/packet_system.cpp#L1362 https://github.com/project-topaz/topaz/blob/release/src/map/status_effect.h#L56

This was most likely done while thinking of trading keys to chests and coffers, which is the correct behavior for them. However, it results in Sneak breaking when trading a Pickaxe to Mine, something it shouldn't.

https://ffxiclopedia.fandom.com/wiki/Sneak_Mining

We can't simply change the flag to the existing EFFECTFLAG_INVISIBLE, because it would ruin the proper behavior for chests.

zach2good commented 3 years ago

We could check for

if (targetOfTrade == a HELM point) { don't remove the stuff}
or
if (targetOfTrade == a HELM point && trade.contains(OnePickaxe)) { don't remove the stuff}
jarmengaud commented 3 years ago

I think we have the same issue when trading an item for a quest For instance for the THF AF in Dangruf, i traded an item to a ??? and it removed sneak and i got instantly killed by a high level worm nearby :-)

zach2good commented 3 years ago

Another way to do this would be to change:

        PChar->StatusEffectContainer->DelStatusEffectsByFlag(EFFECTFLAG_DETECTABLE);
        luautils::OnTrade(PChar, PNpc);

to

    auto result = luautils::OnTrade(PChar, PNpc);
    if (result == don't take away sneak)
    {
        PChar->StatusEffectContainer->DelStatusEffectsByFlag(EFFECTFLAG_DETECTABLE);
    }

Then you could just change the helm.lua onTrade helper to return a value for "don't take away sneak"

zach2good commented 3 years ago

Or like we have helpers that change the message type of an action from Lua, we could modify aspects of the trade while in onTrade

TeoTwawki commented 3 years ago

Seems to me like chests and coffers should not have removed everything detectable via the trade itself core side and instead just had a delStatusEffect() in its global lua so as to spare us this issue of its specific thing firing on other trade events like this.¯\_(ツ)_/¯

ibm2431 commented 3 years ago

1) On retail, isn't it impossible to trade while invisible?

2) If so, can remove flag deletion in 0x036 altogether, and have chests/coffers do it in their scripts

Edit: Teo won by seconds

TeoTwawki commented 3 years ago

Actually I think you do have to drop invis (manually yourself) but only invis, not sneak or deo.

Era-Lusiphur commented 3 years ago

Actually I think you do have to drop invis (manually yourself) but only invis, not sneak or deo.

That is what I recall and backed up with everything I've seen related to it while looking at this initially.

TeoTwawki commented 3 years ago

Got retail confirmation that the client at least universally blocks any ability to attempt trades while invis, so we shoudl disallow trying it rather than stripping it. And chests/coffers are the only thing to remove sneak.