Small problems being that all the items/effect need to be redone and characters now have a default number of attacks of 1/2. Not sure where to change that.
@SethDavis So the opcode now takes the number of APR as its argument, rather than the "key" value? That would be unfortunate, as it leads to portability problems for mods.
@SethDavis I don't suppose it would be possible to keep the significance of the old key values? That way mods won't have to use different values for BGEE. As a bonus, you wouldn't have to chase down the code that makes characters start with key = 1 (1 apr in the old system but 0.5 in the new), or whatever it is.
@Wisp - I suppose I could apply the ToBEx fix if someone could provide it to me. Other than that I'm not seeing a way to fix this without breaking things that count on it being wonky.
I'm not sure how many times this would come up but:
If you keep the old key values wouldn't it lead to problems when using items or spells that would increase your attack per round by 1 or some such? Or would all current items/spells work correctly if the table in the opening post was kept?
@SethDavis I guess I'll ping @Ascension64 for details on the ToBEx fix. I'm not familiar enough with the subject to be relied on for providing the pertinent details.
@Tanthalas If the table in the opening post was kept, it would mean all current uses of this opcode would continue to work as they do now. To my knowledge, the method used by GemRB and ToBEx solves the problem of stacking this effect while still preserving the old key values (meaning, mods do not have to be specifically tailored to any particular implementation).
@SethDavis Too bad numAttacks is a short, otherwise you could just convert the whole field to a float. TobEx works around existing key values by converting short<->float. Feel free to pilfer and modify as necessary... not the most elegant of codes.
float CDerivedStats_NumAttacksShortToFloat(short s) { bool bNegative = s < 0 ? true : false; if (bNegative) s = -s; float f = (float)s;
if (f >= 0.0 && f <= 5.0) { } else { if (f >= 6.0 && f <= 10.0) { f -= 5.5; } else { if (pGameOptionsEx->bDebugVerbose) { LPCTSTR lpsz = "CDerivedStats_NumAttacksShortToFloat(): Number of attacks out of range (%d)\r\n"; console.writef(lpsz, s); L.timestamp(); L.appendf(lpsz, s); } f = 0.0; } }
if (bNegative) f = -f; return f; }
short CDerivedStats_NumAttacksFloatToShort(float f) { short s;
bool bNegative = f < 0.0 ? true : false; if (bNegative) f = -f;
if (f > 5.0) f = 5.0; if (f < 0.0) f = 0.0;
if (f == 0.0 || f == 1.0 || f == 2.0 || f == 3.0 || f == 4.0 || f == 5.0) { s = (short)f; } else { if (f == 0.5 || f == 1.5 || f == 2.5 || f == 3.5 || f == 4.5) { s = (short)(f + 5.5); } else { LPCTSTR lpsz = "CDerivedStats_NumAttacksFloatToShort(): I got a strange number of attacks value (%f)\r\n"; console.writef(lpsz, f); L.timestamp(); L.appendf(lpsz, f); s = 0; } }
@Ascension64 - Thank you very much ^^ This makes way more sense than the way they did it.
Some of these member variables have eluded me thus far, hopefully they're still there.
*begin question barrage* This should be entered into the code right? Not a tp2 or such? Will this play nicely with the weirdness in the weapon stat checks? How did you guys manage to use actual code? *end barrage of questions*
Some of these member variables have eluded me thus far, hopefully they're still there.
The variables are probably different from the source code, since I had to give them all my own names. However, they should all still be there.
This should be entered into the code right? Not a tp2 or such?
Yes, the source code needs to be modified to use the code. I would suggest not including the debug code, or modifying the code to use the debug code relevant to the source, since TobEx uses its own debug implementation.
Will this play nicely with the weirdness in the weapon stat checks?
Which checks are those? The above fix has been in TobEx for quite some time. So far, it has been stable. I can't imagine the code messing up that much because all it does it to make the mathematics occur in float and convert back to short to put back in the existing variables.
How did you guys manage to use actual code?
Oh, by being really naughty. EXE code was replaced/overwritten/modified using code injection.
@SethDavis We also got names of some variables and most functions from assertion texts and debug info from the Mac version. So, if the code is scarily familiar, that's a plausible explanation.
@SethDavis: I do believe there is something inherently incorrect with getting the number of attacks in the above code. I remember looking at the compiled version and it was hard to make sense of because of all the mathematics involved, but haven't done anything about it. Will take a closer look a bit later and give you my opinion.
Agree with snipping that code before release. The TobEx stuff is public, so that can be left in if desired.
@SethDavis: update. I believe the modifications to the number of attacks in your code and my code are independent changes to each other. My code is specifically related to the effect opcode #1 (Number of Attacks mod), whereas your code is specifically related to the proficiency-associated number of attacks bonuses defined in WSPATCK.2DA. Both will make independent changes to the number of attacks in the CDerivedStats struct.
I reviewed the existing BG:EE code you posted and it looks fine (to be nitpicky, there is redundant duplication of code that you could draw out of the if (!offhand) {...} else {...} code block.
As a side note, the code is really sloppy and it and WSPATCK.2DA is really weird to read when it substitutes -1 for +0.5, -2 for +1.5, etc.
So this is... fixed? @Ascension64, is there any straightforward way of testing this- maybe by creating an item that increments up your APR, seeing how it works? As big a fix as it is, it's implemented subtly...
I can confirm--wielding MMM while under OSpin or Boon (with the ApR bonuses, not the shell spell wonk) was holding my ApR steady at 5, instead of rolling over to 1/2.
What about the issue of setting APR with the "Set" parameter and setting APR to "1"? Warrior's APR from Level and proficiency points will be added AFTER the "Set" parameter.
Is there any sort of flag that can be added to the APR bonus to factor in after Level and proficiency bonuses?
And also, has subtracting APR been addressed? It presently does nothing.
@Boaster: I don't think ToB ever features instanteous modifier types like you suggest. TobEx added this to a few opcodes. The fix should handle subtraction and setting.
Comments
So the opcode now takes the number of APR as its argument, rather than the "key" value?
That would be unfortunate, as it leads to portability problems for mods.
I don't suppose it would be possible to keep the significance of the old key values? That way mods won't have to use different values for BGEE. As a bonus, you wouldn't have to chase down the code that makes characters start with key = 1 (1 apr in the old system but 0.5 in the new), or whatever it is.
I'm not sure how many times this would come up but:
If you keep the old key values wouldn't it lead to problems when using items or spells that would increase your attack per round by 1 or some such? Or would all current items/spells work correctly if the table in the opening post was kept?
I guess I'll ping @Ascension64 for details on the ToBEx fix. I'm not familiar enough with the subject to be relied on for providing the pertinent details.
@Tanthalas
If the table in the opening post was kept, it would mean all current uses of this opcode would continue to work as they do now. To my knowledge, the method used by GemRB and ToBEx solves the problem of stacking this effect while still preserving the old key values (meaning, mods do not have to be specifically tailored to any particular implementation).
Too bad numAttacks is a short, otherwise you could just convert the whole field to a float. TobEx works around existing key values by converting short<->float. Feel free to pilfer and modify as necessary... not the most elegant of codes.
float CDerivedStats_NumAttacksShortToFloat(short s) {
Have to fix the effect opcode:bool bNegative = s < 0 ? true : false;
if (bNegative) s = -s;
float f = (float)s;
if (f >= 0.0 && f <= 5.0) {
} else {
if (f >= 6.0 && f <= 10.0) {
f -= 5.5;
} else {
if (pGameOptionsEx->bDebugVerbose) {
LPCTSTR lpsz = "CDerivedStats_NumAttacksShortToFloat(): Number of attacks out of range (%d)\r\n";
console.writef(lpsz, s);
L.timestamp();
L.appendf(lpsz, s);
}
f = 0.0;
}
}
if (bNegative) f = -f;
return f;
}
short CDerivedStats_NumAttacksFloatToShort(float f) {
short s;
bool bNegative = f < 0.0 ? true : false;
if (bNegative) f = -f;
if (f > 5.0) f = 5.0;
if (f < 0.0) f = 0.0;
if (f == 0.0 ||
f == 1.0 ||
f == 2.0 ||
f == 3.0 ||
f == 4.0 ||
f == 5.0) {
s = (short)f;
} else {
if (f == 0.5 ||
f == 1.5 ||
f == 2.5 ||
f == 3.5 ||
f == 4.5) {
s = (short)(f + 5.5);
} else {
LPCTSTR lpsz = "CDerivedStats_NumAttacksFloatToShort(): I got a strange number of attacks value (%f)\r\n";
console.writef(lpsz, f);
L.timestamp();
L.appendf(lpsz, f);
s = 0;
}
}
if (bNegative) s = -s;
return s;
}
BOOL CEffectAttacksPerRoundMod::ApplyEffect(CCreatureObject& creTarget) {
...and the + operator for CDerivedStats.switch (effect.nParam2) {
case 0: //add
if (effect.nTiming == 1) {
float fNumAttacks = CDerivedStats_NumAttacksShortToFloat(creTarget.m_BaseStats.numAttacks) + CDerivedStats_NumAttacksShortToFloat((short)(effect.nParam1));
creTarget.m_BaseStats.numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
bRefreshStats = TRUE;
bPurge = TRUE;
} else {
float fNumAttacks = CDerivedStats_NumAttacksShortToFloat(creTarget.cdsDiff.numAttacks) + CDerivedStats_NumAttacksShortToFloat((short)(effect.nParam1));
creTarget.cdsDiff.numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
bPurge = FALSE;
}
break;
case 1: //set
if (effect.nTiming == 1) {
creTarget.m_BaseStats.numAttacks = effect.nParam1;
if (creTarget.m_BaseStats.numAttacks > 10) creTarget.m_BaseStats.numAttacks = 10;
if (creTarget.m_BaseStats.numAttacks < 0) creTarget.m_BaseStats.numAttacks = 0;
bRefreshStats = TRUE;
bPurge = TRUE;
} else {
creTarget.cdsCurrent.numAttacks = effect.nParam1;
if (creTarget.cdsCurrent.numAttacks > 10) creTarget.cdsCurrent.numAttacks = 10;
if (creTarget.cdsCurrent.numAttacks < 0) creTarget.cdsCurrent.numAttacks = 0;
bPurge = FALSE;
}
break;
case 2: //percent
if (effect.nTiming == 1) {
float fNumAttacks = CDerivedStats_NumAttacksShortToFloat(creTarget.m_BaseStats.numAttacks) * (float)effect.nParam1 / 100.0;
fNumAttacks = floor(fNumAttacks * 2.0 + 0.5) / 2.0; //round to nearest 0.5
creTarget.m_BaseStats.numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
if (creTarget.m_BaseStats.numAttacks < 1) creTarget.m_BaseStats.numAttacks = 1;
bRefreshStats = TRUE;
bPurge = TRUE;
} else {
float fNumAttacks = CDerivedStats_NumAttacksShortToFloat(creTarget.m_BaseStats.numAttacks) * (float)effect.nParam1 / 100.0;
fNumAttacks = floor(fNumAttacks * 2.0 + 0.5) / 2.0; //round to nearest 0.5
creTarget.cdsCurrent.numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
if (creTarget.cdsCurrent.numAttacks < 1) creTarget.cdsCurrent.numAttacks = 1;
bPurge = FALSE;
}
break;
default:
LPCTSTR lpsz = "DETOUR_CEffectAttacksPerRoundMod::DETOUR_ApplyEffect(): invalid effect.nParam2 (%d)\r\n";
console.writef(lpsz, effect.nParam2);
L.timestamp();
L.appendf(lpsz, effect.nParam2);
bPurge = TRUE;
break;
}
return TRUE;
}
CDerivedStats& CDerivedStats::operator+(CDerivedStats& cds) {if (pGameOptionsEx->bEffAttacksPerRoundFix) {
float fNumAttacks = CDerivedStats_NumAttacksShortToFloat(numAttacks) + CDerivedStats_NumAttacksShortToFloat(cds.numAttacks);
numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
cds.numAttacks = 0;
numAttacks = CDerivedStats_NumAttacksFloatToShort(fNumAttacks);
}
...
}
Some of these member variables have eluded me thus far, hopefully they're still there.
*begin question barrage*
This should be entered into the code right? Not a tp2 or such?
Will this play nicely with the weirdness in the weapon stat checks?
How did you guys manage to use actual code?
*end barrage of questions*
Agree with snipping that code before release. The TobEx stuff is public, so that can be left in if desired.
I reviewed the existing BG:EE code you posted and it looks fine (to be nitpicky, there is redundant duplication of code that you could draw out of the if (!offhand) {...} else {...} code block.
As a side note, the code is really sloppy and it and WSPATCK.2DA is really weird to read when it substitutes -1 for +0.5, -2 for +1.5, etc.
I hate to use floats where integers are enough.
Just made ITMs that increase APR, using a 1st lvl Fighter w/ 1 pip in Longsword and a wielding a regular Longsword (SW1H04) and a base APR of 3/2: As a bonus it maxed out at 5... rather than wrapping or falling to 0 or something... Just sayin'...
Whatever was done, certainly seems to work- and integrates with the native NI system for ITM creation. :-)
Which ToBEx number does this APR fix correspond with? I'm intending to file this thusly.
Is there any sort of flag that can be added to the APR bonus to factor in after Level and proficiency bonuses?
And also, has subtracting APR been addressed? It presently does nothing.
The fix should handle subtraction and setting.