carxt / JohnnyGuitarNVSE

JohnnyGuitar is a library extending NVSE's functionality. Please check the README provided for more information.
GNU Lesser General Public License v2.1
55 stars 9 forks source link

SetOnActorValueChangeEventHandler #34

Closed WalkerMx closed 2 years ago

WalkerMx commented 2 years ago

This is related to the bug here, where the event fires constantly with the infinitesimal changes in the Actor Value.

Looking in JohnnyEvents.h, line 157: if (previousVal = 0.0) previousVal = g_thePlayer->avOwner.GetActorValue(avCode);

Should that use equality, not assignment? if (previousVal == 0.0) previousVal = g_thePlayer->avOwner.GetActorValue(avCode);

Also, on line 159: if (previousVal != newVal) {

Is it necessary to compare them as floating-point values? I'm not sure if any Actor Values are greater than zero but less than one, so rounding to 1-2 decimal places might work. Even just plain rounding fixes the runaway event firing: if (round(previousVal) != round(newVal)) {

I don't do much C++ programming, so I might be wrong, or these might not be the best solutions. I hope this is helpful.

c6-dev commented 2 years ago

Good catch, thanks.

c6-dev commented 2 years ago

Unfortunately there are other issues with it. This approach only works when the value is ModAV'd, if it's Set or Forced the output is nonsensical. Not sure how to go about fixing this.

WalkerMx commented 2 years ago

Well, like I mentioned, I don't really do C++ much, but this seems to correct the output when using ForceAV and SetAV:

void __stdcall HandleAVChangeEvent(int avCode, float previousVal, float modVal) {
    if (previousVal == 0.0) previousVal = g_thePlayer->avOwner.GetActorValue(avCode);
    float newVal = previousVal + modVal;
    if (newVal != g_thePlayer->avOwner.GetActorValue(avCode)) newVal = modVal;
    if ((round(previousVal) != round(newVal))) {
        for (auto const& callback : OnAVChangeHandler->EventCallbacks) {
            if (reinterpret_cast<JohnnyEventFiltersOneFormOneInt*>(callback.eventFilter)->IsInFilter(1, avCode)) {
                CallUDF(callback.ScriptForEvent, NULL, OnAVChangeHandler->numMaxArgs, avCode, *(UInt32*)&previousVal, *(UInt32*)&newVal);
            }
        }
    }
}

I've just added a line to check the supposed 'newVal' against what the value actually is, according to GetAV. If the values aren't the same, we assume someone used SetAV or ForceAV, and set newVal to modVal.

Certainly not the prettiest, but it seems to work, at least with Health, DamageThreshold, and Speech.

c6-dev commented 2 years ago

That didn't work at all for me. image

WalkerMx commented 2 years ago

Let's see.. So, some of the weirdness I believe is due to Skills being partially a Derived Statistic.

SetAV won't change the base points or points from Tag!, I believe, so doing SetAV 0 would reset the skill back to it's base value, plus any points from the Tag! perk. ModAV can adjust the base stats, and ForceAV locks the value to whatever you pass it.

All that said, you're right, something wrong is still happening. It looks like, previousVal is always 0 when calling this on a Skill.

void __stdcall HandleAVChangeEvent(int avCode, float previousVal, float modVal) {
    if (previousVal == 0.0) previousVal = (g_thePlayer->avOwner.GetActorValue(avCode) - modVal);
    float newVal = previousVal + modVal;
    if (newVal != g_thePlayer->avOwner.GetActorValue(avCode)) newVal = modVal;
    if ((round(previousVal) != round(newVal))) {
        for (auto const& callback : OnAVChangeHandler->EventCallbacks) {
            if (reinterpret_cast<JohnnyEventFiltersOneFormOneInt*>(callback.eventFilter)->IsInFilter(1, avCode)) {
                CallUDF(callback.ScriptForEvent, NULL, OnAVChangeHandler->numMaxArgs, avCode, *(UInt32*)&previousVal, *(UInt32*)&newVal);
            }
        }
    }
}

Because the Skills are derived, I'm not sure if I can get the accuracy to 100% on my own. Let me know if this code improves anything.

WalkerMx commented 2 years ago

After looking at it for a while, this seems to work, and is less ugly:

void __stdcall HandleAVChangeEvent(int avCode, float previousVal, float modVal) {
    float newVal = g_thePlayer->avOwner.GetActorValue(avCode);
    if (previousVal == 0.0) previousVal = (newVal - modVal);
    if ((round(previousVal) != round(newVal))) {
        for (auto const& callback : OnAVChangeHandler->EventCallbacks) {
            if (reinterpret_cast<JohnnyEventFiltersOneFormOneInt*>(callback.eventFilter)->IsInFilter(1, avCode)) {
                CallUDF(callback.ScriptForEvent, NULL, OnAVChangeHandler->numMaxArgs, avCode, *(UInt32*)&previousVal, *(UInt32*)&newVal);
            }
        }
    }
}

Still, because ModAV, SetAV, and ForceAV behave differently, the values aren't always what you expect. SetAV and ForceAV both now work in the technical sense, however changing Actor Values like this is bound to cause other problems.

Let me know what you think.

c6-dev commented 2 years ago

Thanks for your trouble, i'll check later. I also have an option of adding more hooks to account for differences, although that's its own pain.