Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPCs draw weapon on dead monsters #13

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

NPCs draw weapon on dead monsters #13

wants to merge 1 commit into from

Conversation

szapp
Copy link
Collaborator

@szapp szapp commented Mar 31, 2021

NPCs sometimes draw their weapon even if the monster is already dead.

@AmProsius AmProsius removed ai labels Jan 1, 2021
@AmProsius AmProsius added the validation: required This issue needs validation from one of the validators. label Jan 28, 2021
@AmProsius
Copy link
Owner Author

PrintGlobals (PD_ZS_CHECK);
//######## Ist NSC eine WACHE oder BOSS ? ########

changed to

	PrintGlobals			(PD_ZS_CHECK);

	if C_NpcIsDown(other)
	{
		PrintDebugNpc		(PD_ZS_CHECK, "...Monster kampfunfähig!");
		return;
	};

	//######## Ist NSC eine WACHE oder BOSS ? ########

@AmProsius AmProsius added the provided fix This issue has a fix provided in the comments. label Feb 16, 2021
@AmProsius AmProsius added this to the v1.1.0 milestone Mar 5, 2021
@szapp szapp added the impl: hook script func This issue requires hooking script functions. label Mar 17, 2021
@szapp szapp added compatibility: easy This issue is easy to make compatible. type: session fix The fix for this issues is persistent across a session. labels Mar 17, 2021
@szapp
Copy link
Collaborator

szapp commented Mar 18, 2021

The provided fix might not fully work, because the NPC would remain in that AI state and continue to the state loop after the return. An AI_ContinueRoutine(self) before the return would work. UPDATE: I think it wouldn't the NPC would re-detect the monster every few seconds and restart their daily routine every time. Not an option.

Conceptually, I would like to stick to the mentioned assumptions in the comment block above the function, and not even let the state be started in the first place. That would require to find out which of the callers cause the bug:

// Dieser Zustand wird durch
//
// - B_AssessEnemy -> NSC entdeckt feindliches Monster
// - B_AssessFighter -> NSC entdeckt Monster im Kampfmodus (trifft auf alle zu!)
// - B_AssessFightSound -> NSC hört Monster kämpfen
// - B_AssessWarn -> NSC wird vor Monster gewarnt (VORSICHT: Noch nicht überarbeitet!)
// - B_ObserveIntruder -> NSC wird von Monster überrascht
//
// gestartet. Folgende Bedingungen werden ebenfalls vorausgesetzt:
//
// - Das Monster ist aggressiv und würde den NSC angreifen!
// - Das Monster befindet sich innerhalb von 'HAI_DIST_ASSESS_MONSTER'

@szapp szapp added compatibility: difficult This issue is difficult to make compatible. and removed compatibility: easy This issue is easy to make compatible. labels Mar 18, 2021
@szapp szapp self-assigned this Mar 18, 2021
@szapp szapp added impl: unknown There is no clear plan on how to implement this issue yet. and removed impl: hook script func This issue requires hooking script functions. labels Mar 18, 2021
@szapp
Copy link
Collaborator

szapp commented Mar 29, 2021

@AmProsius, I think I am gonna need an example case where that happens, in order to work on it.

There are so many callers (notes to myself):

  • B_AssessEnemy
  • B_AssessFighter
  • B_AssessFightSound <-- Intercept call to AI_StartState, check for ZS_AssessMonster and return if monster is down
  • B_AssessWarn <-- Intercept call to AI_StartState, check for ZS_AssessMonster and return if monster is down
  • B_ObserveIntruder
  • B_AssessEnterRoom <-- Intercept call to AI_StartState, check for ZS_AssessMonster and return if monster is down
  • ZS_AssessDefeat <-- Intercept call to AI_StartState, check for ZS_AssessMonster and return if monster is down
  • ZS_AssessMurder <-- Intercept call to AI_StartState, check for ZS_AssessMonster and return if monster is down
  • ZS_AssessMonster <-- should be safe
  • ZS_MCHunting <-- should be safe

@szapp szapp removed the provided fix This issue has a fix provided in the comments. label Mar 29, 2021
@N1kX94
Copy link

N1kX94 commented Mar 30, 2021

In Gothic Mod Fix, this function is rewritten like this:

func void ZS_AssessMonster()
{
	C_ZSInit();
	
	// Идентификатор "other" не инициализирован (загрузка сохранения) -> выход.
	if(!Hlp_IsValidNpc(other))
	{
		AI_ContinueRoutine(self);
		return;
	};
	
	// К моменту старта состояния враг уже мёртв -> выход.
	if(C_NpcIsDown(other))
	{
		AI_ContinueRoutine(self);
		return;
	};
	
	Npc_SendPassivePerc(self,PERC_ASSESSWARN,other,self);
	
	// Остановка, быстрый поворот к врагу.
	B_FullStop(self);
	B_WhirlAround(self,other);
	
	// Непись является представителем стражи или боссом -> старт состояния атаки.
	if(C_NpcIsGuard(self) || C_NpcIsGuardArcher(self) || C_NpcIsBoss(self))
	{
		B_Say_Monster(self,other);
		B_SetAttackReason(self,AR_GuildEnemy);
		Npc_SetTarget(self,other);
		AI_StartState(self,ZS_Attack,0,"");
		return;
	};
	
	// Непись слабее врага -> старт состояния бегства.
	if(C_AmIWeaker(self,other))
	{
		Npc_SetTarget(self,other);
		db_say(self,NULL,"ShitWhatAMonster");
		AI_StartState(self,ZS_Flee,0,"");
		return;
	};
	
	// Непись является партнёром по группе -> старт состояния атаки.
	if(self.aivar[AIV_PartyMember])
	{
		B_Say_Monster(self,other);
		B_SetAttackReason(self,AR_GuildEnemy);
		Npc_SetTarget(self,other);
		AI_StartState(self,ZS_Attack,0,"");
		return;
	};
	
	// Активация восприятий.
	Npc_PercEnable(self,PERC_ASSESSMAGIC,B_AssessMagic);
	Npc_PercEnable(self,PERC_ASSESSDAMAGE,B_AssessDamage);
	Npc_PercEnable(self,PERC_ASSESSTALK,B_RefuseTalk);
	Npc_PercEnable(self,PERC_ASSESSSURPRISE,B_AssessSurprise);
	
	// !!!!! Другие восприятия в состоянии????? 
	
	// Извлечение оружия.
	B_DrawWeapon(self,other);
	
	// Время ожидания.
	self.aivar[AIV_StateTime] = Hlp_Random(100)%3 + 3;
};

func int ZS_AssessMonster_Loop()
{
	//B_PrintAIInfo("ZS_AssessMonster_Loop",5,8,1);
	
	// Выход, если враг уже мёртв.
	if(C_NpcIsDown(other))
	{
		return LOOP_END;
	};
	
	// Выход, если расстояние до врага превышает 18м.
	if(Npc_GetDistToNpc(self,other) > HAI_DIST_ABORT_ASSESS_MONSTER)
	{
		return LOOP_END;
	};
	
	// Непись находится в режиме дальнего боя или магического боя -> старт состояния атаки.
	if(Npc_IsInFightMode(self,FMODE_FAR) || Npc_IsInFightMode(self,FMODE_MAGIC))
	{
		Npc_SetTarget(self,other);
		B_SetAttackReason(self,AR_GuildEnemy);
		AI_StartState(self,ZS_Attack,0,"");
	};
	
	// Расстояние до врага стало меньше 10м или время ожидание превысило отведённое значение -> старт состояния атаки.
	if((Npc_GetDistToNpc(self,other) < HAI_DIST_ATTACK_MONSTER) || (Npc_GetStateTime(self) > self.aivar[AIV_StateTime]))
	{
		Npc_SetTarget(self,other);
		B_Say_Monster(self,other);
		B_SetAttackReason(self,AR_GuildEnemy);
		AI_StartState(self,ZS_Attack,0,"");
	};
	
	B_SmartTurnToNpc(self,other);
	B_SelectWeapon(self,other);
	AI_Wait(self,0.5);
	return LOOP_CONTINUE;
};

func void ZS_AssessMonster_End()
{
	AI_StopLookAt(self);
	B_RemoveWeapon(self);
	self.aivar[AIV_StateTime] = 0;
};

@szapp
Copy link
Collaborator

szapp commented Mar 31, 2021

I think then it should suffice to add a check for C_NpcIsDown(other) in both ZS_AssessMonster and ZS_AssessMonster_Loop to resolve the specific bug discussed here.

The test of this fix is not working yet.

Refs #13
@szapp szapp marked this pull request as draft March 31, 2021 09:34
@szapp szapp removed their assignment Mar 31, 2021
@szapp
Copy link
Collaborator

szapp commented Mar 31, 2021

The test is not functional yet. I cannot reproduce the problem, and need an example of how to trigger the bug.

@szapp szapp added compatibility: easy This issue is easy to make compatible. impl: hook script func This issue requires hooking script functions. and removed compatibility: difficult This issue is difficult to make compatible. impl: unknown There is no clear plan on how to implement this issue yet. labels Apr 5, 2021
@szapp
Copy link
Collaborator

szapp commented Apr 5, 2021

This bug still has to be validated. Whoever does this, when doing so, please provide instruction on how to reproduce the bug, such that the test can be adjusted.

@szapp
Copy link
Collaborator

szapp commented Apr 17, 2021

Moved to v1.2.0, because of unclear circumstances of the bug.

@szapp
Copy link
Collaborator

szapp commented Jul 18, 2021

What's left to do here is to clarify the circumstances that cause this bug. I could not reproduce it. As I never experienced it I don't know when it happens. Someone else will have to provide that information. I will have to remove myself from being assigned.

@szapp szapp removed their assignment Jul 18, 2021
@AmProsius AmProsius modified the milestones: v1.2.0, v1.3.0 Oct 30, 2021
@AmProsius AmProsius removed this from the v1.3.0 milestone Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility: easy This issue is easy to make compatible. impl: hook script func This issue requires hooking script functions. type: session fix The fix for this issues is persistent across a session. validation: required This issue needs validation from one of the validators.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants