Try / OpenGothic

Reimplementation of Gothic 2 Notr
MIT License
1.15k stars 82 forks source link

Earthquake can be heard from large distance #305

Closed Dehela closed 3 months ago

Dehela commented 2 years ago

Looks like there is an issue with earthquake, which happens near temple ruins which leads to Jharkendar, that it can be heard from large (or even unlimited) distance. Like, starting the game in Xardas Tower, you can already hear it from time to time.

I suppose it might something to do with full life simulation (like mentioned in FPS ticket) for this version so the event happens despite the distance. It case if it matters, I was testing master branch on Linux.

Try commented 2 years ago

I suppose it might something to do with full life simulation

Almost: actual root cause of earthquakes in game is Saturas. In his state-script he has 5% chance to summon global earth-quake.

Script callstack:

"WLD_PLAYEFFECT"
"B_EVENT_PORTAL_EARTHQUAKE"
"ZS_STUDY_WP_LOOP"
--- 
Npc::tickRoutine
Dehela commented 2 years ago

So, is it actually a normal thing to happen? I haven't play the game for quite a while so I might recall things wrong. Or the point was that Saturas summons those earthquakes normally but because original game doesn't process NPCs who are far away, you would experience earthquakes only when you are close to Saturas to make him be active for mischiefs?

Try commented 2 years ago

I'm also not 100% sure how it was in vanilla.

So far my point is:

  1. Earthquakes are related to npc-processing
  2. It' likely that original game doesn't execute npc-state-scripts, if npc is outside of certain distance
  3. Script is crap to begin with :)
Dehela commented 2 years ago

Sounds like a complicated issue to fix unless things are altered. Probably replacing global effect with ranged one (if it is possible) is the least evil, which is unfortunate if you aim to keep game logic intact as it was in original (but might be not too bad if it's not the first time to make a workaround patch).

PolyMeilex commented 2 years ago

Random idea, perhaps we could check source of the effect and fade it smoothly based on position, so instead of unloading like gothic did it woul be a lot nicer and slowly decay over distance.

sirsteve1 commented 2 years ago

I suppose it might something to do with full life simulation

Almost: actual root cause of earthquakes in game is Saturas. In his state-script he has 5% chance to summon global earth-quake.

Script callstack:

"WLD_PLAYEFFECT"
"B_EVENT_PORTAL_EARTHQUAKE"
"ZS_STUDY_WP_LOOP"
--- 
Npc::tickRoutine

Dear Try, I tried to find the callstack of the earthquake routine but had no chance of finding anything. In which files would I have to lookup? I would like to play around and see if I can find a solution for this. Checked out both the OpenGothic repo as well as the Gothic2 game folder but found nothing...

Try commented 2 years ago

Hi, @sirsteve1 !

Npc::tickRoutine - npc.cpp WLD_PLAYEFFECT - gamescript.cpp (GameScript::wld_playeffect) B_EVENT_PORTAL_EARTHQUAKE- script_g2\Content\Story\Events\B_Event_Portal_EarthQuake.d (not in opengothic repo - you need original game scripth here) ZS_STUDY_WP_LOOP - script_g2\Content\AI\Human\TA_Human\ZS_Study_WP.d

Pinemarten commented 3 months ago

It' likely that original game doesn't execute npc-state-scripts, if npc is outside of certain distance

Indeed, the vanilla Gothic controls which NPCs are enabled and which are not based on the distance to the player (his camera). Therefore, the routines of NPCs that are far away are not processed. The earthquake effect can be triggered by any one the mages' routines which are near the portal to Jarkendar.

You can see the API (in original Gothic) that controls it that I used back in 2011: https://github.com/Pinemarten/GothicMultiplayer/blob/main/GMP_Client/g2api/ocspawnmanager.hpp

I believe that in order to be compatible with vanilla scripts, OpenGothic should also only process routines of NPCs that are close to the player. There's some code that already assesses the distance of NPCs to the player (see Npc::ProcessPolicy::AiFar). Perhaps we could not execute routines of NPCs that have Npc::ProcessPolicy::AiFar or at least Npc::ProcessPolicy::AiFar2 set?

@Try, what do you think?

Try commented 3 months ago

Looking at this as well in to https://github.com/Try/OpenGothic/pull/653, I think we need to summarize cases first and agree on how to address problem overall and not particular cases.

So the issue: full world simulation results in significant behavior differences between OpenGothic and Gothc2.

Why full world simulation:

Known issues

Perhaps we could not execute routines of NPCs that have Npc::ProcessPolicy::AiFar or at least Npc::ProcessPolicy::AiFar2 set?

At least aiState.funcIni has to be invoked in order for npc to progress and walk to next point in theirs time-table. I'm assuming in vanilla, if npc is away, from player engine will move it automatically. Also need to handle auto move/teleport for Npc_ExchangeRoutine. So not calling aiState and handle npc movement in engine manually is an option: fixing many issues, except #585 . For #585 need something more advanced.

One thing to note here, I wish orks outside the castle not to stand in idle pose, but be more organic. This requires to invoke aiState.funcIni at least once for all npc's, what unfortunately doesn't go along with proposed scheduling, but it's ok.

thokkat commented 3 months ago
* Earthquake (this issue)

  * Essentially any other global call from script, akin to `WLD_PLAYEFFECT` will cause trouble

Maybe don't execute commands which produce a sound or effect that can confuse the player. Like it's done for svm-talks and keep gloabal script.

* NPC problems

  * NPCs won't heal in time

* Npc state doesn't refresh

Motivation for pr was that templars in sleeper temple missing equip command in script. Testing showed that best weapons are equipped every time npc comes in visibility range. At same time script is called to restore health.

I think handling these two in Npc::resetPositionToTA which covers sleep and world change missing only the "npc is now visible" call makes most sense.

* Maybe more?

Not more but cases where workarounds/hacks are already in place.

Pedro removal inNpc::startState, same as Greg/Dexter.

  if(wp=="TOT" && aiPolicy!=Player && aiPolicy!=AiNormal) {
    // workaround for Pedro removal script
    auto& point = owner.deadPoint();
    attachToPoint(nullptr);
    setPosition(point.position());
    }

No call to state loop for far away monsters in Npc::tickRoutine

  /*HACK: don't process far away Npc*/
  if(aiPolicy==Npc::ProcessPolicy::AiFar2 && routines.size()==0)
    return;
Try commented 3 months ago

Closing in favor of https://github.com/Try/OpenGothic/issues/657