Fundamental Boolean expression evaluation defect

[EDGvyyBUS9aZpT6054IJ]

As a preface I want to say that I'm quite skeptical that this is something that Obsidian isn't already aware of but I can't find any existing publications about this(Forums search or Google.) and it would be a shame if this didn't ever get fixed and has been around for quite a while now it seems so I'm trying posting this here.

In abstract terms, when Boolean expressions are evaluated(In dialogue files and globalscripts called from dialogue at least. Others I haven't tested.) the 2nd type of logical operator used within a given parenthetical(In Deadfire JSON terms I'm talking about ConditionalExpressions, including the top level of the expression which isn't labelled as such in the JSON) but not inside a descendant parenthetical triggers the rest of the expression to be silently ignored.

To give some examples with a C-like Boolean expression format:
"A || B && C" is affected by this because the logical AND is the second type of operator used after logical OR with the result that the expression is evaluated as though it were "A || B".
"A || (B && C)" is not affected by this because the second type of logical operator is found only in a descendant parenthetical.
"A || B || C" is not affected by this because there is only one type of logical operator used.
"A && (B && C || D)" is affected by this because the logical OR in the descendant parenthetical is the second type of logical operator used in the descendant parenthetical after logical AND with the result that this expression is evaluated as though it were "A && (B && C)"
In the given examples A-D may be parentheticals or basic expression components without affecting the occurrence of the problem.

Now you know why I'm skeptical that Obsidian patched the whole game every time since at least 1.2.0.0017(The oldest version i tested for this.) while remaining unaware of such a fundamental defect.

The most readily perceptible example of which I'm aware of this causing a problem in the game is when you try to break into VTC headquarters in Neketaka you can only use acid keyword abilities instead of acid and fire keyword abilities to melt the bars as is seemingly intended. This is because in the globalscripts the relevant expression is "A && B || C" where A is "!AbilityIsPassive()", B is "AbilityHasKeyword(Acid)", and C is "AbilityHasKeyword(Fire)" and as described the expression gets evaluated as "A && B". If you modify the expression to be "A && C || B" then fire keyword abilities work and acid ones don't. As a bonus defect it seems likely the intent behind that expression was "A && (B || C)" though I'm not aware of any practical impact from that.

I have verified that this defect is present in versions 1.2.0.0017(Once again: I haven't tested anything older. Note the VTC bars example is NOT present in this version because the expression in that version was actually "B || C" if you refer to the definitions used in the prior paragraph. However, modifying a pair of dialogue responses' conditions to something like "AlwaysTrue() && AlwaysFalse() || AlwaysTrue()" and "(AlwaysTrue() && AlwaysFalse()) || AlwaysTrue()" results in only one of the responses appearing which verifies this defect exists in that version.), 3.1.1.0023, and 4.0.0.0034 and have verified that this defect occurs on 2 different computers(An old Windows 7 system and a recent Windows 10 system) so I suspect that this defect is evident on many if not all installations of the game that are patched to at least 1.2.0.0017.

 

Based on analyzing the JSON files in version 4.0.0.0034 there are 33 expressions in dialogue presumably affected in some way by this and 30 in expressions in other kinds of files.

[EDGvyyBUS9aZpT6054IJ]

More examples of how this defect perverts the apparent intent behind Boolean expressions that currently exist in the game files:

lax1_exported\design\conversations\lax01_00_arena_island\lax01_00_si_crossing.conversationbundle: node #45 ("[Leap to catch [unathletic party member]'s wrists.]")
The expression for this response to be available is "IsAttributeScoreValue(Player, Dexterity, >=, 16, False) && HasAbility(Player, Wild_Sprint) || HasAbility(Player, Leap) || HasAbility(Player, Dragon_Leap) || HasAbility(Player, Panthers_Leap) || HasAbility(Player, Charge) || HasAbility(Player, Long_Stride)" and as those who've been paying attention will know already this will be evaluated as "IsAttributeScoreValue(Player, Dexterity, >=, 16, False) && HasAbility(Player, Wild_Sprint)" making this option far less available than is seemingly intended. You may have also noticed that this is another bonus defect since if this were evaluated properly simply satisfying "HasAbility(Player, Leap) || HasAbility(Player, Dragon_Leap) || HasAbility(Player, Panthers_Leap) || HasAbility(Player, Charge) || HasAbility(Player, Long_Stride)" would be enough to make the option available irrespective of the player's dexterity. The ability checks should be grouped.

lax1_exported\design\conversations\lax01_00_arena_island\lax01_00_cv_big_boar.conversationbundle: node #5 ("You look hungry, fella. Here, maybe I've got something you can eat.")
The expression for this response to be available is "IsItemCount(Plant_Orlans_cradle, >=, 1) || IsItemCount(Food_pork, >=, 1) || (IsItemCount(Plant_Huona_mahe, >=, 1) && IsSkillValue(Player, Alchemy, >=, 4, False, False) || IsSkillValue(Player, Survival, >=, 5, False, False))" and it will be evaluated as "IsItemCount(Plant_Orlans_cradle, >=, 1) || IsItemCount(Food_pork, >=, 1) || (IsItemCount(Plant_Huona_mahe, >=, 1) && IsSkillValue(Player, Alchemy, >=, 4, False, False))" removing the option for player characters that have non-party assisted survival >= 5 and not non-party assisted alchemy >= 4 and a certain combination of items when it was clearly intended by the human(s) behind the expression to be available to them. This is another bonus defect since if this were evaluated properly the option could be available simply by having non-party assisted survival >= 5 and none of the items due to the order of operations.

lax1_exported\design\conversations\lax01_00_arena_island\lax01_00_cv_arena_weapon_seller.conversationbundle: node #10 ("Are you a member of the Darcozzi Paladini?")
The expression for this response to be available is "!HasConversationNodeBeenPlayed(lax01_00_cv_arena_weapon_seller, 13) && HasSubClass(Player, Paladin_DarcozziPaladini) || IsCulture(Player, OldVailia)" and it will be evaluated as "!HasConversationNodeBeenPlayed(lax01_00_cv_arena_weapon_seller, 13) && HasSubClass(Player, Paladin_DarcozziPaladini)". Less impactful than the prior 2, this one simply denies player characters from Old Valia who aren't Darcozzi paladins one line from the merchant and a possible interjection from Pallegina before converging with the generic "How did you come to be here?" response. This is another bonus defect since if this were evaluated properly the option could be available simply by the player character being from Old Vailia and ignoring whether or not node #13 had been played.

Are you noticing a pattern to which expressions seem to be affected by this? It's almost as though Obsidian were aware of this defect and only ran afoul of it accidentally while apparently failing to ever mention it to modders. These accidents also show off why this would be a seriously bad thing for those modding the game since this tends to obscure wrong expressions in addition to perverting the intent of those wrongly assuming Deadfire would evaluate Boolean expressions the same as every other bit of technology does.

[EDGvyyBUS9aZpT6054IJ]

For the record I'm going through the list of expressions in reverse alphabetical path order since the dialogue files ended up much closer to the end than the beginning of the alphabetically path sorted list of files and expressions I generated making it easier to keep my place initially. Yes, it's very arbitrary. No, these were not cherry picked in any way other than being dialogue expressions that fit the description specified in the original post.

More examples of how this defect perverts the apparent intent behind Boolean expressions that currently exist in the game files:

exported\design\conversations\re_scripted_interactions\re_si_supply_wagon_trap.conversationbundle: node #223 ('[Attack] "Maia?"')
The expression for this response to be available is "IsCompanionActiveInParty(CompanionMaia) && (IsWeaponTypeEquippedInPrimarySlot(CompanionMaia, Arquebus) || IsWeaponTypeEquippedInPrimarySlot(CompanionMaia, Pistol) && IsWeaponTypeEquippedInPrimarySlot(CompanionMaia, Blunderbuss))" and it will be evaluated as "IsCompanionActiveInParty(CompanionMaia) && (IsWeaponTypeEquippedInPrimarySlot(CompanionMaia, Arquebus) || IsWeaponTypeEquippedInPrimarySlot(CompanionMaia, Pistol))". Actually in this case the evaluation defect makes this expression more functional than it would otherwise be since nobody can equip both a pistol and blunderbuss in their primary slot and at least this way the pistol is considered an option. Still the intent was clearly that blunderbusses should also be an option and the fact that the operator to its left is && instead of || is another example of this defect being triggered only due to a mistake when formulating the expression.

exported\design\conversations\re_scripted_interactions\re_si_shrine_worshipers.conversationbundle: node #121 ("The Huana share a few pensive glances before the leader among them nods...")
The expression for this first non-player-response line we've reached to appear (Only coming into play if none of its higher placed alternatives' condition expressions are fulfilled, as with most non-player-response nodes) is "(AreGuidsSameObject(Specified4, Player) && IsCulture(Player, DeadfireArchipelago) || IsSkillValue(Specified4, Religion, >=, 6, True, False)) || IsSkillValue(Specified4, Religion, >=, 7, True, False)" and it will be evaluated as "(AreGuidsSameObject(Specified4, Player) && IsCulture(Player, DeadfireArchipelago)) || IsSkillValue(Specified4, Religion, >=, 7, True, False)". What they apparently meant to do here was give the player from the Deadfire Archipelago a lowered religion skill requirement of 6 instead of 7(oddly miserly difference) which would be the requirement for any party member not the player or not from the Deadfire Archipelago. This expression is another case of Obsidian not formulating it correctly to begin with and it getting further mutilated by the evaluation defect as a direct result. The || left of the 6 religion check should have been &&. Without the evaluation defect the effect of this expression would have been that any party member not the player or not from the Deadfire Archipelago would need a religion skill of 6 while the player character from the Deadfire Archipelago would need no particular religion skill due to the inappropriate ||. The trailing religion 7 check would literally never come into play. However with the evaluation defect the player character from the Deadfire Archipelago still requires no particular religion skill but any other party member reaching this node's evaluation(i.e. not Tekehu who's specifically filtered by a higher placed alternative) will require 7 religion skill.

exported\design\conversations\re_scripted_interactions\re_si_plague_ship_dead_tiny.conversationbundle: node #6 ("It takes some time, but you manage to clear the blockage and open up the hold...")
The expression for this non-player-response line to appear(Same deal as the immediately prior one relative to responses.) is "AreGuidsSameObject(Specified5, Player) && IsAttributeScoreValue(Specified5, Might, >=, 14, True) || IsSkillValue(Specified5, Mechanics, >=, 8, True, False)" and it will be evaluated as "AreGuidsSameObject(Specified5, Player) && IsAttributeScoreValue(Specified5, Might, >=, 14, True)". The apparent intent here is to first require the player to be the one interacting then require that they either have at least 14 might or at least 8 mechanics. Due to a lack of parentheses around the attribute/skill requirements what we get before the evaluation defect is that this line is conditional upon it being the player interacting and having at least 14 might OR anybody is interacting and they have at least 8 mechanics(and the node that is supposed to catch a non-player character with at least 8 mechanics is linked lower than this one so nothing's stopping that from happening on that front). After the evaluation defect what we get is that only a player character with at least 14 might can get this line, which does at least prevent the game referring to a non-player-character as 'you'.

So that's another 3 on top of the 4(including the 1 in the original post) previous expressions that all have the same pattern of a single simple mistake(wrong operator or missing pair of parentheses) in formulating the expression directly leading to the evaluation defect triggering. I suppose it's to be expected that errors of that kind in sufficiently lengthy expressions would tend to create the circumstances required if they didn't already exist by other means. Still the very low fraction of expressions showing the signature makes me think there must have been something that artificially lowered the number without pushing it to zero. Like a policy about not trusting people to remember the logical operator order of operations that wasn't totally automated for some bizarre reason and was therefore fallible. Amusing to consider that the evaluation defect itself could be considered a totally automated, yet in its silent truncation utterly perverse, enforcement of such a policy.

[SChin]

Hey there!

Whelp, I can't pretend I have any idea what any of this means, but I'll put a report together and send it to the Programming team to investigate this.  If you have any more you'd like to contribute to this particular issue, feel free to continue posting it here as I will be linking this thread to the report so the programmers can reference this.

 

Thank you for the help!

[AndreaColombo]

Very detailed analysis. Hope these can get fixed!

[MaxQuest]

Hats off for a detailed report.

It's a rare sight from a first poster with a gibberish nickname)

 

ConditionalExpressions, including the top level of the expression which isn't labelled as such in the JSON) but not inside a descendant parenthetical triggers the rest of the expression to be silently ignored.

 

To give some examples with a C-like Boolean expression format:

"A || B && C" is affected by this because the logical AND is the second type of operator used after logical OR with the result that the expression is evaluated as though it were "A || B".

Ha! I knew that it's a bad idea to use different logical operators inside a single ConditionalComponents array (if you use 3 or more components), because without parentheses the order is unclear.

 

For example was modding a "[not in combat] or [stealthed] or [invisible]". And had a random thought of what would I do in a situation when I would need both [or] and [and]? Figured that perhaps using multiple Conditionals is the way... but it's parent (in my case ActivationPrerequisites) is an object not array:

 

 

 

"ActivationPrerequisites": {
  "Conditional": {
    "Operator": 1,
    "Components": [
      {
        "$type": "OEIFormats.FlowCharts.ConditionalCall, OEIFormats",
        "Data": {
          "FullName": "Boolean IsInCombat()",
          "Parameters": [],
          "Flags": "",
          "UnrealCall": "",
          "FunctionHash": -1740638672,
          "ParameterHash": -1740638672
        },
        "Not": true,
        "Operator": 1
      },
      {
        "$type": "OEIFormats.FlowCharts.ConditionalCall, OEIFormats",
        "Data": {
            "FullName": "Boolean IsCharacterStealthed(Guid)",
            "Parameters": ["011111e9-0000-0000-0000-000000000000"],
            "Flags": "",
            "UnrealCall": "",
            "FunctionHash": 1739009647,
            "ParameterHash": 643611138
        },
        "Not": false,
        "Operator": 1
      },
      {
        "$type": "OEIFormats.FlowCharts.ConditionalCall, OEIFormats",
        "Data": {
          "FullName": "Boolean HasStatusEffectWithKeyword(Guid, Guid)",
          "Parameters": [
            "011111e9-0000-0000-0000-000000000000",
            "fe80a668-dcb3-4cea-ae2e-70d47dc91348"
          ],
          "Flags": "",
          "UnrealCall": "",
          "FunctionHash": -1392641717,
          "ParameterHash": 1237200400
        },
        "Not": false,
        "Operator": 1
      }
    ]
  }
}

 

Edited December 24, 2018 by MaxQuest

[EDGvyyBUS9aZpT6054IJ]

Boy, I was all set to keep this on the front page while everybody ignored my wonderful post for another 4 days or so. Congratulations, to everybody finally replying!

[EDGvyyBUS9aZpT6054IJ]

MaxQuest(And anybody else who really wants to understand.), the order of operations is CRYSTAL CLEAR if you aren't incapable of remembering that && is evaluated before || and there are only 2 such binary logical operators so you don't have to remember anything else! The order of evaluation is NOT the problem! Except when Obsidian left out the parentheses. Then it is their problem but distinct from the evaluation defect.

More examples of how this defect perverts the apparent intent behind Boolean expressions that currently exist in the game files:

exported\design\conversations\re_scripted_interactions\re_si_crew_event_high_money.conversationbundle: node #19 ('"So you've said, captain." [greedy-if-available crew member] frowns over crossed arms. "Yet here we are."')
The expression for this non-player-response line to appear is "!HasConversationNodeBeenPlayed(re_si_crew_event_high_money, 26) && HasConversationNodeBeenPlayed(re_si_crew_event_high_money, 17) || HasConversationNodeBeenPlayed(re_si_crew_event_high_money, 18)" and it will be evaluated as "!HasConversationNodeBeenPlayed(re_si_crew_event_high_money, 26) && HasConversationNodeBeenPlayed(re_si_crew_event_high_money, 17)". Impossible to tell just from the expression(Readable code? What's that?) but the apparent goal here was to establish whether the player had previously threatened but not actually so far used violence in previous instances of this event occurring. Yet another pair of missing parentheses mean that before the evaluation defect this would have been true in the case of any successful threat even if the player had actually used violence previously. With the evaluation defect only a failed threat would count but at least you'd get credit for the violence reliably.

exported\design\conversations\companions\companion_tekehu_wants_to_talk.conversationbundle: node #2 ("For what did you do it, captain??")
The expression for this non-player-response line to appear is "IsGlobalValue(n_Queen_Onezaka_State, ==, 5) || IsGlobalValue(n_Prince_Aruihi_State, ==, 5) && IsCompanionActiveInPartyOrPresent(CompanionTekehu)" and it will be evaluated as "IsGlobalValue(n_Queen_Onezaka_State, ==, 5) || IsGlobalValue(n_Prince_Aruihi_State, ==, 5)". The disappointing drip drop of missing parentheses continues. Before the evaluation defect this would have checked whether Onezaka or Aruihi had been killed and incorrectly only required Tekehu to be present in the case of Aruihi being dead. After the evaluation defect this just doesn't care at all whether Tekehu is present.

exported\design\conversations\companions\companion_tekehu_wants_to_talk.conversationbundle: node #8 ("I say that I sleep better when you are around, captain.")
The expression for this non-player-response line to appear is "IsGlobalValue(n_Tekehu_Romance, >, 0) && IsGlobalValue(n_Tekehu_Romance, <, 6) && IsGlobalValue(n_Tekehu_Relationship_Mood, >, 0) || IsCompanionActiveInPartyOrPresent(CompanionTekehu)" and it will be evaluated as "IsGlobalValue(n_Tekehu_Romance, >, 0) && IsGlobalValue(n_Tekehu_Romance, <, 6) && IsGlobalValue(n_Tekehu_Relationship_Mood, >, 0)". The first file with 2 matching expressions! Trying to make sure there's a player-Tekehu romance happening and something about 'relationship mood' and that Tekehu is present, this is a case of || instead of && left of the IsCompanionActiveInPartyOrPresent and before the evaluation defect would result in this being satisfied merely with Tekehu being present OR if the romance were happening. After the evaluation defect it would only be satisfied when the romance was happening and wouldn't care at all whether Tekehu was present. Without the evaluation defect this would presumably be much more noticeable in game since it would cause the node to play merely because Tekehu was present(although only once due to the "persistence" property) and replace one of the 2 lower placed alternatives while with it it presumably either effectively doesn't show the line at all since Tekehu isn't anywhere you could see it or only plays it when the romance is happening so the player's experience wouldn't be something very surprising to them. Actually now that I think about it when I once lured Bertenno away from the cutscene zone in the Dark Cupboard with a sparkcracker and talked to him to trigger the otherwise apparently vestigial dialogue of his that has the player attempting without the possibility of success to stop him from running off and he then did leave before his cutscene ever played and then I entered the cutscene zone and the result was that the nearest party member to Fassina spoke Bertenno's lines so I'm not actually certain what this would look like in game. There's a bonus defect for you though I'm not too sure how many people would try throwing a sparkcracker in there before the cutscene without just trying to break the game.

So now it's up to 10 cases of this being triggered due to missing parentheses or swapped operators and 0 of it being triggered by anything else.

[EDGvyyBUS9aZpT6054IJ]

While looking at one of the nodes featured in this post I uncovered something that perhaps no-one except the person who wrote it has ever seen(Now that I think about it the translators must have seen it but likely had little understanding of the significance lore-wise.) and which is so lore-inappropriate that it passes the mark for defect status by a country mile in my book.

If the player character is a priest of Eothas, Wael, Berath, or Skaen and you fail every single opportunity to save Bearn(All but impossible unless you deliberately try, which combined with the priest and deity requirements probably explains why no player seems to have ever seen this and gone on to talk about it) then you can just verbally ask your god to save Bearn and then... divine intervention occurs. Not spell-casting. Not Watcher-powers. No ability checks. DIVINE! INTERVENTION! With a levitating corpse, signs from above, poison-vomit-snakes, and resurrection of the dead(Except for Skaen naturally. He just plays with the corpses. But it is still divine intervention without mistake.). I'm going to paste in one excerpt but you really should experience it fully if you possibly can without playing through the game as a priest just for one version of it. Maybe modify the dialogue to remove the priest checks and load a save around there or teleport with IRoll20s and F11(Fair warning: I've never tried that.)? It's up to you. Now the excerpt(from Wael's version): "Then, lifelessly, Bearn's body is brought to its feet as if by invisible strings. His mouth opens, and reddish-brown liquid floats outward from it in the shape of a snake. The snake slithers to the ground. It blinks at you with many eyes, then dissolves instantly into a puddle.".

Now, some of you might be thinking "It's a fantasy game. The player is an exceptional individual and a priest of [deity]. What's so wrong here?". The answer is that this game's story hinges absolutely on the idea that the gods are almost totally unwilling to intervene in anything directly whatsoever when it's within the material world. Even when one of them like Eothas goes off and starts pushing the envelope by possessing this or that they still don't start levitating things or anything along those lines. What they ultimately do in round 1 of the Eothas problem is get multiple exceptional followers from their different religions together(Including the party member Durance from POE1 if that jogs any memories.) and then the followers build an unprecedented bomb and blow Eothas to smithereens by luring him onto a bridge where the bomb is concealed. What they do in round 2 of the Eothas problem is send the player to deal with him. And as you may have noticed they don't directly help you out with that other than when you are in the spirit realm which they intervene in all the time(Keep in mind, the display at the Neketaka docks is presented as Berath somehow influencing your own character's poorly defined Watcher-powers, presumably via the chime(Remember that thing in your chest from the intro?) rather than whatever is supposed to be happening here.) Of course just recently Obsidian undermined this argument for me with The Forgotten Sanctum and Wael but he still very much has to be wheedled into it under unprecedented circumstances and even then he possesses something as Eothas does instead of doing anything like this. And then they directly intervene in the material world just to save Bearn(Or what Skaen does, which is even harder to justify however much it might agree with his sadistic tendencies since there isn't really any plausible long-term effect.) just. Because. You. Asked. Was there anything else your character might have wanted to ask for?

On top of all that, in Berath's version it's written as though you sent the Sun-in-Shadow souls back to The Wheel without actually checking what the save says about that when as far as I know there's no such requirement for priests of Berath even if you had to stay the same kind of priest or a class in general between games, which you don't.

More examples of how this defect perverts the apparent intent behind Boolean expressions that currently exist in the game files:

exported\design\conversations\companions\companion_tekehu_main.conversationbundle: node #199 ("So... here we are. What's on your mind?")
The expression for this response to be available is "IsGlobalValue(n_Tekehu_Romance, ==, 3) && IsInScene(Player, AR_0311_Vailian_District_Tavern_01) || !HasConversationNodeBeenPlayed(companion_tekehu_relationship_player, 276)" and it will be evaluated as "IsGlobalValue(n_Tekehu_Romance, ==, 3) && IsInScene(Player, AR_0311_Vailian_District_Tavern_01)". The || left of HasConversationNodeBeenPlayed should be &&. Again HasConversationNodeBeenPlayed makes it impossible to tell from the expression alone but the node it's checking to have not been played is part of an alternate version of this scenario. The original expression could cause this node to play just because you hadn't seen the alternate version of the scenario(Wouldn't THAT be easily spotted as bad behaviour!). The evaluation defect version of the expression could conceivably cause both versions of the scenario to play if you talked to Tekehu in the Wild Mare again(Not something you'd be likely to do. Harder to notice.).

exported\design\conversations\companions\companion_eder_quest_ship.conversationbundle: node #124 ("[Administer taru taru combined with ripple sponge.]")
The expression for this response to be available is "IsSkillValue(Player, Alchemy, >=, 4, True, False) && IsItemCount(Drug_Taru_Turu_Chew, >=, 1) || IsItemCount(Drug_Ripple_Sponge, >=, 1)" and it will be evaluated as "IsSkillValue(Player, Alchemy, >=, 4, True, False) && IsItemCount(Drug_Taru_Turu_Chew, >=, 1)". Once more the missing parentheses. The effect is that this could be available simply because you have a ripple sponge on you no matter what your alchemy or chew-having status is. Except the evaluation defect makes it so that instead you do need alchemy and chew but it doesn't care if you have ripple sponge or not. Mixed bag.

exported\design\conversations\companions\companion_cv_talkingweapon01.conversationbundle: node #115 ("Clearly there's more to this than you're letting on. Tell me what happened - maybe I can help.")
The expression for this response to be available is "IsSkillValue(Player, Diplomacy, >=, 2, True, False) || IsDisposition(BenevolentDisposition, Rank1, >=) && IsDisposition(HonestDisposition, Rank1, >=) && IsDisposition(DiplomaticDisposition, Rank1, >=)" and it will be evaluated as "IsSkillValue(Player, Diplomacy, >=, 2, True, False) || IsDisposition(BenevolentDisposition, Rank1, >=)". GET HYPE PEOPLE! IT'S THE FIRST MATCHING EXPRESSION THAT ISN'T NECESSARILY WRONG! If this expression weren't in Deadfire it would be perfectly acceptable! Mind you, in this same conversation file there's another skill-or-dispositions condition that has any one out of 3 dispositions(and therefore doesn't trigger the evaluation defect due to only using ||) instead of all of the 3 dispositions to satisfy the condition without any apparent rationale for the difference so it's not entirely clear that this is totally deliberate(and no condition in any other dialogue file even uses at least 2 disposition checks with a skill/attribute check so don't look there for something to make this un-weird) but if it isn't it at least isn't a matter of a single swapped operator or missing parentheses and I'm marking this as not wrong as far as the original formulation goes. I confess that when I first looked at this one I started trying to decide whether it was a swapped operator or missing parentheses before even considering it might be anything else. But of course you know that in this thread, THERE ARE NO HAPPY ENDINGS! Thanks to the evaluation defect the only disposition that really matters is benevolent disposition! I guess that's kind of happy?

exported\design\conversations\companions\companion_cv_talkingweapon01.conversationbundle: node #138 ("I don't think this is really about me.")
I'm throwing this one in here because it's a near-clone of node #115 including not necessarily being wrong. Once again, the only disposition that really matters is benevolent. Yes, this isn't the all || one alluded to earlier, there are 3 like this in this one file and nowhere else.

From 0-2 not necessarily wrong expressions in one post(Though I'm tempted to only count it as 1 since they're near-clones.). And the 2 wrong-due-to-single-swapped-operator-or-missing-parentheses expressions also found in this post bring the total to 12 of those so far.

Post script: This was the last of the content I had queued up. I'd imagined doing more but now that the thread is being taken seriously my motivation has evaporated. I really hope I don't need to tell Obsidian to thoroughly review the rest of the matching expressions.

In closing I want to say that after seeing the before/after of the evaluation defect in practice on a small sample of Obsidian-style expressions I now believe that the defect has been in the game since well before release(still haven't tested the release version though, since I can't bear to install that again.(The last time was to check whether "The Dragon Thrashed..." had been utterly ruined since release or not after I tested it out and was disgusted. Has anybody else noticed that it actually alternates damage types in Deadfire on top of the armour problems? It definitely just did both at once in POE1. Also no animation. Really? Also you should really really add a way to see damage over time damage in the log with detailed explanations like with other attacks. I've had a bad time trying to figure out where the numbers came from for Toxic Strike and still have doubts about it but at least now I know that most of that is due to sneak attack (And damage bonuses from the wielded weapon.) applying to the DOT effect on it even though that doesn't work with the DOT from Strike the Bell's one-handed version, apparently because only AttackStatusEffectsIDs get the bonuses and Strike the Bell (And some other attack abilities IIRC.) use a system of applying an effect to the attacker which then applies an effect to the target in turn.)) since if the evaluation defect had been introduced after most expressions were written some stuff would have been conspicuously broken.

I also really believe now that Obsidian had no idea about this as an organization (which you should not be any more proud of since this is really really dumb stuff to find in 4.0 of anything having been there since at least 1.2, never mind in 2018. And having "tuned" the durations of player abilities down to the second long before noticing it.) as this really does tend to conceal wrong expressions on top of for some reason rarely meeting the required conditions in the corpus. It certainly seems likely to me that some people at Obsidian may have actually noticed this and are laughing at the rest of you behind your backs so be suspicious. Then again maybe Obsidian just runs everybody ragged enough that when they see something that doesn't make sense to them they just shrug instead of investigating.

Post post script: It's dumb to make the player choose between a limited number of disposition adding choices with a conditional response the only escape from it. Also, did you know that no abilities actually have affliction keywords and the relevant function for filtering abilities can't find the ones on the status effects inflicted? Also, sorry for cramming so many distinct ideas into one thread. I couldn't bother with lesser defects before and may well never post again. I think you'd do well to read carefully to compensate, however.

[MaxQuest]

MaxQuest(And anybody else who really wants to understand.), the order of operations is CRYSTAL CLEAR if you aren't incapable of remembering that && is evaluated before || and there are only 2 such binary logical operators so you don't have to remember anything else! The order of evaluation is NOT the problem! Except when Obsidian left out the parentheses. Then it is their problem but distinct from the evaluation defect.

Ah, that's good

 

I mean, I know that boolean conjunction takes precedence over disjunctive in boolean algebra.

But have little idea about how boolean expressions are actually evaluated during runtime. I remember hearing some fuzzy info about the short-circuit evaluations, and the left-to-right approach where the next argument is evaluated only if the previous wasn't false while being an operand in && function (and viceversa: if the previous wasn't evaluated to true while being an operand in || function), and that there can be some nuances in different languages. So just decided to use as a rule: use parentheses! if you want to be clear in intent and sure in result.

 

The most readily perceptible example of which I'm aware of this causing a problem in the game is when you try to break into VTC headquarters in Neketaka you can only use acid keyword abilities instead of acid and fire keyword abilities to melt the bars as is seemingly intended. This is because in the globalscripts the relevant expression is "A && B || C" where A is "!AbilityIsPassive()", B is "AbilityHasKeyword(Acid)", and C is "AbilityHasKeyword(Fire)" and as described the expression gets evaluated as "A && B". If you modify the expression to be "A && C || B" then fire keyword abilities work and acid ones don't. As a bonus defect it seems likely the intent behind that expression was "A && (B || C)" though I'm not aware of any practical impact from that.

Do I understand it right, that if Obsidian dropped the parentheses in A && (B || C), the expression had to be rewritten as A && B || A && C?

 

Also you should really really add a way to see damage over time damage in the log with detailed explanations like with other attacks. I've had a bad time trying to figure out where the numbers came from for Toxic Strike and still have doubts about it but at least now I know that most of that is due to sneak attack (And damage bonuses from the wielded weapon.) applying to the DOT effect on it even though that doesn't work with the DOT from Strike the Bell's one-handed version, apparently because only AttackStatusEffectsIDs get the bonuses and Strike the Bell (And some other attack abilities IIRC.) use a system of applying an effect to the attacker which then applies an effect to the target in turn.)) since if the evaluation defect had been introduced after most expressions were written some stuff would have been conspicuously broken.

There is something similar with Soul Annihilation damage.

It deals phys_dmg and raw_dmg. This raw part - is so to say spell damage which is increased in function of current focus. Yet it somehow gets increased by Sneak Attack, Savage Attack (weapon proficiency), Weapon Quality, Two Handed Style and such.

Edited December 24, 2018 by MaxQuest

[BMac]

Yes, the game evaluates Boolean expressions purely left-to-right (EDIT: right-to-left) and has no operator precedence. This was probably done because it was much quicker to implement, and we can still create whatever behavior we want by using parenthesis.

 

(Personally, agree with MaxQuest that relying on operator precedence hides the creator's intent and is harder to read, and I always use parenthesis in my code when it comes up)

 

It's a bad idea for us to change such fundamental code this late in the game, but we'll get bugs in the system for the cases where parentheses need to be added to expressions to make them work right. Thanks for those detailed lists!  I'll also add a note in the modding documentation for expressions.

[EDGvyyBUS9aZpT6054IJ]

Somebody please explain to me how "AlwaysTrue() && AlwaysFalse() || AlwaysTrue()" evaluates to false even with "Purely left-to-right and has no operator precedence". Or is it just like I said and part of the expression is silently ignored?

"Those detailed lists" were not exhaustive and if you want to fix your game you have to do some trivial JSON parsing to get the list of affected expressions, just in case you thought otherwise.

I really can't believe you're talking about leaving it like this and sabotaging modders forever instead of fixing the problem and rechecking 63 affected expressions.

[BMac]

I took a look at the code and determined that expressions are evaluated right-to-left and, yes, there is a bug with evaluating that specific formula (and also its inverse).  We'll take a look and see what we can do.

 

Modders who are creating complex boolean expressions won't have any trouble if they use a nested ConditionalExpression (i.e. parenthesis) to clarify their intent when mixing And and Or operators.

[EDGvyyBUS9aZpT6054IJ]

Whew, what a relief I was only dealing with a lack of reading comprehension.

I had this screed all written up about how Obsidian shot themselves in the foot SIXTY-THREE TIMES with date ranges and the number of times Obsidian patched the game without noticing this and everything and how Obsidian was slothfully just going to put up a sign somewhere to warn modders not to do the same.

Something I was thinking about floating if fixing the problem wasn't an option in certain peoples' minds was maybe just outputting a warning to the log if the game decides your expression needs truncating.

Seriously though it's all in the OP people, the rest is just so you know how serious the problem is.

Please read the OP.

[EDGvyyBUS9aZpT6054IJ]

So to put an end to all the rampant speculation in the thread about when this defect was really introduced I finally got around to testing the other POE-likes and they are ALL AFFECTED and ALL HAVE AFFECTED EXPRESSIONS!

POE1 3.7.0.1280: 19 total but 2 of them seem unlikely to do anything so I'm going to call it 17 which look misbehaviour-inducing.
Tyranny "gold" 1.2.1.0160: 33 total, every one of which looks misbehaviour-inducing.
Torment: Tides of Numenera 1.1.0: 15 total, every one of which looks misbehaviour-inducing.

What are you going to do, Obsidian? Release special updates for the games you can? Make "You should get yourself checked" phone calls? Stop selling the games you can(LOL)? Post lame disclaimers? Pretend this isn't happening and count on any lame disclaimers you already posted?