Mordrag can be told to "get out" in New Camp

[AmProsius]

Describe the bug
Mordrag can be told to "get out of here" even if he guided the player to the New Camp.

Expected behavior
Mordrag can no longer be told to "get out of here" if he guided the player to the New Camp.

Additional context
Bug provided by Blubbler.

Reference for the proposed fix:

gothic-1-community-patch/scriptbase/_work/Data/Scripts/Content/Story/Missions/DIA_ORG_826_Mordrag.d

Lines 328 to 334 in e66b303

	FUNC int Org_826_Mordrag_Fight_Condition()
	{
	if ( (Thorus_MordragKo == LOG_RUNNING) && (!Npc_KnowsInfo(hero,Org_826_Mordrag_GotoNewcamp)) )
	{
	return 1;
	};
	};

[AmProsius]

gothic-1-community-patch/scriptbase/_work/Data/Scripts/Content/Story/Missions/DIA_ORG_826_Mordrag.d

Lines 395 to 404 in e66b303

	FUNC int Org_826_Mordrag_HauAb_Condition()
	{
	VAR C_NPC Mordrag;
	Mordrag = Hlp_GetNpc(Org_826_Mordrag);
	if (Mordrag.aivar[AIV_WASDEFEATEDBYSC] >= 1)
	{
	return 1;
	};
	};

changed to

FUNC int  Org_826_Mordrag_HauAb_Condition()
{
    VAR C_NPC Mordrag;
    Mordrag = Hlp_GetNpc(Org_826_Mordrag);

    if (Mordrag.aivar[AIV_WASDEFEATEDBYSC] >= 1)
    && (Thorus_MordragKo == LOG_RUNNING)
    && (!Npc_KnowsInfo(hero,Org_826_Mordrag_GotoNewcamp))
    {
        return 1;
    };
};

[szapp]

@AmProsius this will need some fixing.

[szapp]

To cover all ends, this test is incomplete. The fix introduces two new or-conditions for which the dialog condition should fail, but the dialog condition is only tested here once, for both of them at the same time. The test should have two passes (i.e. calling the dialog condition twice), for either of the two conditions. Only that will fully cover testing the fix.

[AmProsius]

The fix introduces two new and-conditions, so I don't see the problem here. Or am I mistaken?

[szapp]

Yes, you are right. The dialog condition returns true if both new conditions are met, but the dialog condition returns false if either of the new conditions if false. The test currently confirms that the dialog condition returns false if the dialog "Org_826_Mordrag_GotoNewcamp" was told. What the test should also check is, that the dialog condition also returns false if only the variable "Thorus_MordragKo" is not set to LOG_RUNNING.

[AmProsius]

Ah I see. Thanks for the explanation, I will update the test accordingly.

[szapp]

Thanks!

[AmProsius]

Okay, thanks for the review! I based this fix and test on #153 (fix of #33), so I guess I'll have to update that fix and test as well.

[szapp]

Good catch. The fix of #33 is slightly different. Here we have two independent and-conditions (if either is false return false), in #33 we have two or-conditions (if both are false return false), compare this with this.

It may not hurt to negate the conditions in #33, but also does not seem necessary (there are no double negations). The symbol check in #33 is important and should remain. Unlike here, the existence of the symbol has to be checked again, because it is not guaranteed that it exists:

gothic-1-community-patch/src/Ninja/G1CP/Content/Fixes/Session/fix033_ShrikeQuestDialog.d

Line 6 in f386ca4

&& ((G1CP_IsInfoInst("DIA_Gorn_Hut")) || (G1CP_IsIntVar("Gorn_ShrikesHut", 0))) { // Either one must exist

As for the test of #33, it does not need two passes, because there are or-conditions. However, you are right, this line should be adjusted as mentioned above:

gothic-1-community-patch/src/Ninja/G1CP/Content/Tests/test033.d

Line 24 in f386ca4

G1CP_SetIntVarI(varId, 0, 0);