Infinite disposition via MWDialogue::Filter::testDisposition() glitch
Found an interesting glitch yesterday that essentially got me a Telvanni guard with "infinite" (constantly reset to 100 upon dialogue open) disposition.
Reproducer:
Finish the "Gateway haunting" quest in Sadrith Mora (or set Journal "town_Sadrith" to >= 65).
Find a Telvanni member in Sadrith Mora (e.g. a guard).
setdisposition 100
#* Any value >= 100 works. #* I don't know how I got a disposition >= 100 with the guard in the first place, but it's a prerequisite for this bug.
Enter a dialogue with the NPC.
Lower your disposition to < 70 (a few rounds of Persuasion).
Select the "Gateway haunting" topic. Every time you click it, you'll get 20 disposition.
#* This will not be reflected by the disposition bar; i.e. it will never reach 100. #* This topic has two candidates, one that does nothing and is triggered if disposition is >= 70, the other (fallback) adds 20 disposition. #* The intended logic behind this topic (and vanilla behaviour, judging from reading UESPWiki) is that you can raise your disposition up to 70-89, but not further.
Exit the dialogue. You now have >>100 baseDisposition with that NPC, so every time you open the dialogue, it's displayed as 100.
This can be repeated ad infinitum to train Speechcraft. Just open the dialogue, run Persuasion until your displayed disposition is < 70, then hammer the "Gateway haunting" topic to your liking.
It's a bit of a complex bug, even more so because there are multiple candidate bugs involved in this and you get to choose which one is the undesirable one ;)
- An actor's baseDisposition can be > 100. If this is a bug, it should be prevented in setBaseDisposition() (and possibly even getBaseDisposition()).
- MWDialogue::Filter::testDisposition(), which is used to filter the candidate topics, tests against derivedDisposition. This is a value bounded between [0, 100]. It also takes mTemporaryDispositionChange into account.
int actorDisposition = MWBase::Environment::get().getMechanicsManager()->getDerivedDisposition(mActor) + MWBase::Environment::get().getDialogueManager()->getTemporaryDispositionChange();
In the specific case presented above, testDisposition() is used to test @actorDisposition > 70@. As soon as your mTemporaryDispositionChange in this dialogue is < -30, the check will always fail (because derivedDisposition can only be <= 100, so actorDisposition is always < 70). This means that you will trigger the fallback topic, which includes a ModDisposition 20 script (which raises baseDisposition even further, but has no influence on derivedDisposition - it will still be 100). Lather, rinse, repeat, get thousands of baseDisposition and train Speechcraft at the same time. This could be fixed by using the actor's baseDisposition instead of derivedDisposition in testDisposition().
I wrote a little patch that solves it, but it does so by accident - essentially, I moved the "setBaseDisposition() and reset mTemporaryDispositionChange/mPermanentDispositionChange" portion of DialogueManager::goodbyeSelected() into a new function called applyPermanentDispositionChange() and call that during DialogueManager::executeTopic(). It works and doesn't impact performance too much, but it's definitely the wrong fix.
I'll gladly contribute a patch, but you have to decide which way to solve it, because I've only spent about 4 hours in the OpenMW codebase, all of them last night while trying to figure this one out ;) The risk amounts to breaking disposition checks in other topics. If baseDisposition is supposed to go over 100 and it no longer does, it breaks stuff. If testDisposition() checks against baseDisposition instead of derivedDisposition, it no longer accurately reflects the disposition bar (which AFAICT uses derivedDisposition) and might break stuff that relies on this behaviour.
NB: While writing this report, I also noticed that it's sufficient for baseDisposition to be 100, which is definitely an acceptable value (so limiting setBaseDisposition() to <= 100 would not solve this).
(RM-3233 from redmine: created on 2016-03-04 by Manuel Acanthephyra, , closed on 2016-03-17 by Manuel Acanthephyra)