BG2: Make haste APR bonus work properly by moving it from FXOpcodes to Actor #1733
Conversation
gemrb/core/Scriptable/Actor.cpp
Outdated
| } else { // normal haste | ||
| apr += 2 - apr % 2; // +1, round down | ||
| } | ||
| Modified[IE_NUMBEROFATTACKS] = apr; // do NOT call SetStat or it will be clamped |
There was a problem hiding this comment.
Working on Modified here is fine, but the comment shouldn't be needed. Just increase the limit in the maximum_* array.
There was a problem hiding this comment.
Then anything will be able to exceed 5 APR, which is not the original game behavior. E.g. level 13 fighter with two stars dual-wielding Belm and Kundane would have 1 + 0.5 + 2 + 1 + 1 = 5.5 APR, but the original game clamps them to 5. But haste, and haste alone, can exceed that to 6 or 10. I've commented above that wspatck bonuses are properly clamped by gemrb, which is correct behavior. But haste is, and must be, an exception.
There was a problem hiding this comment.
Ok, then I suggest you just remove the comment.
There was a problem hiding this comment.
I can do that, if you don't think it's helpful. No big deal.
|
Are ranged weapons supposed to be able to be boosted? I guess it's the difference between the original "fixed" mode and tobex's final mode. Yes, IE_ATTACKNUMBERDOUBLE is misnamed, not misused, but it's a waste of a stat either way. Slight speedup to keep it, but that's all. And wow, boots of speed indeed affect APR! A tob-starting fighter is the easiest way to check. If on a reload item effects are queued later, this means other things will now be broken. Boots of speed would disable the APR doubling of improved haste. |
|
There's some inconsistency whether ranged APR is supposed to be affected by wspatck (I've read it isn't in EE? I don't have EE), but here's a proof fixed APR weapons are affected by haste: Energy Blades ( That stat is not a waste of stat with my PR, as it governs the haste APR bonus. I highly suspect it does so too in the original game. Item effects were shifted on reload for as long as I remember, is that not intended? It's annoying sometimes (e.g. DUHM and giant belts or dex gauntlets), but not critical. Boots are problematic, yes. Perhaps don't have them unset |
|
I don't see anything special in CREImporter that would break the ordering on load/save. However, I can easily imagine that only persists for a little bit, until we apply equipping effects when adding the inventory. Minor nuisance, I wouldn't touch it. I suggest you convert IE_IMPROVEDHASTE to a bitfield. That way states can be anded together by effects, separately checked for, and IE_ATTACKNUMBERDOUBLE can be repurposed (very valuable property). |
|
While the idea is very tempting, doesn't it have the potential to break mods/scripts that use |
|
I doubt any do, especially the second one, but I've asked the community to weight in. The base check people are doing is definitely with states, but let's see, maybe SCS cares. |
|
I'll be waiting then. The backup solution is to give weak haste the lowest priority -- only have it set |
|
No response, but I've checked SCS. It's using the improved haste stat only by checking if it's not zero: |
|
But if I make A safer solution would be to flip Alternatively, for SCS compatibility in particular, it checks if the stat is greater than zero. Then if improved is 1, normal is 0, and weak is -1, the check would still work correctly and we can cramp it all in one stat. |
|
Yeah, but if you set to a fixed amount, effects will still override each other. What I had in mind was that normal haste doesn't touch it, improved BORs (let's say) 1 and weak BORs 2. Normal mode can be detected by checking if the stat is 0 and STATE_HASTED is on, so the same I just double-checked and the effect bonuses are indeed not cumulative. Casting improved haste doubled the rate without the boots. So you're also right that improved haste should be checked for first. |
|
I got the idea, but then scripts will think weak haste is improved, which is not perfect. Probably tolerable, as the former is rare, but what I meant for the (1, 0, -1) variant is roughly this pseudocode: old = -2;
if (state & STATE_HASTED)
old = stat(IMPROVEDHASTE);
state |= STATE_HASTED;
switch (haste_type) {
case 0:
new = 0;
break;
case 1:
new = 1;
break;
case 2:
new = -1;
break;
}
if (new > old)
stat(IMPROVEDHASTE) = new;That way they will override each other in the proper order 1 > 0 > 2. |
|
You're right, better limit the special logic to one place, no need to pollute the trigger code as well. |
|
I'm doing that last thing I posted then? |
|
Yes please. |
It used to be set in the effect queue, but that doesn't really work as, for one, the fixed APR of most launchers would override it (items come after spells following a reload). Instead we're storing haste type in the IE_IMPROVEDHASTE stat, and then there's a block late in actor stat refresh code that manually adjusts APR based on that stat. Further, type 0 and type 2 haste are supposed to round the APR to integers, which they didn't before. And, by being applied manually and last, haste can now exceed the limit of 5 APR, which it should.
|
Here, the priorities seem to work fine now. By the way, it's now fairly easy to make the -2 weapon speed bonus not stackable. Is it supposed to be? |
|
Thanks! And yeah, pretty sure the speed bonus should be cumulative. |
All forms of haste computed attacks per round way incorrectly, especially combined with other APR modifiers.
Belm and Kundane didn't work properly with improved haste for the same reason.
It's also after wspatck handler code now, which helps, although because it sets base it would only be wrong by one tick anyway. (Incidentally, setting base also explains how that code respects the limit of 5 APR despite not calling
ClampStat().)Also, isn't
IE_ATTACKNUMBERDOUBLEmisnamed? It's for weak haste, which does not double anything.It's all in IESDP. Normal haste rounds down, weak haste rounds up. Normal and improved haste can break the limit, weak can't because of math. I suppose if anything else messed with the APR stat later that tick it could get clamped again, but evidently nothing does. I have got code that enforces a variable limit through PCF, but I ended up not needing it.