coavins / mhrise-coavins-dps

A detailed DPS meter for Monster Hunter Rise (PC)
GNU General Public License v3.0
24 stars 11 forks source link

Game seemingly freezes on quest complete? #18

Closed liu2g closed 2 years ago

liu2g commented 2 years ago

Describe the bug On quest completion with the mod enabled, game freezes completely until I kill it

Steps to Reproduce For example:

  1. Install mod v2.2.0 and dependencies
  2. Configure mod with Script generated UI
  3. Enter a quest hosted by someone else
  4. Game freezes immediately at the exact moment quest completes, just before the kill cam activates. The freeze can only be killed by killing the process.

Expected behavior Game should not freeze when quest completes

REFramework commit hash (if known): e44aa472

Additional context I admit I only tried this twice before I disabled the mod. Then the game does not freeze anymore. See the uploaded gist for my config file.

coavins commented 2 years ago

Thanks for including the save file, I will try to reproduce this. I haven't seen this issue yet in my testing.

Can you list any other mods that you had installed, whether or not they're using REFramework?

coavins commented 2 years ago

I did some testing this morning and was able to get a freeze, not at quest completion but when reaching the monster after joining an ongoing quest.

I suspect these crashes might be related to the method I'm using to track ailment damage. I will investigate to see if there are other options here that would be safer to use.

coavins commented 2 years ago

I have released v2.2.1 which rolls back the poison and blast damage tracking until these performance issues can be resolved.

praydog commented 2 years ago

You need to be careful where you call getHpVital and potentially other similar functions from. This value is encrypted, which is why "Monster Has HP Bar" was calling it within the monster update method hook to decrypt and cache it.

It seems to only be callable at certain points within a tick/frame, otherwise an exception is thrown and potentially undefined behavior can occur. The code implementation for getHpVital looks fairly complex, as a side note.

This is just from a glance at the code, and is my first guess. Could be something else.

coavins commented 2 years ago

At the time these crashes/FPS drops were reported, I was hooking into both activate and update on snow.enemy.EnemyConditionDamageParamBase which has something like 8-10 instances (at least) on every large monster, and possibly small monsters too. I assumed this was the cause of the issue and removed those hooks. In 6498b40 I moved this into the enemy update function which is already being hooked in this script for other purposes.

I'm a little concerned that I'm starting to do too much work in the enemy update given that it presumably runs every frame, but so far the performance issues seem to be resolved. I feel pretty confident that it was the update hook on that condition object which was the problem.

I will stop using getHpVital altogether if you think there is reason to avoid it, there's probably something better on EnemyCharacterBase anyway that I ought to be using instead. I don't need the HP values, I just need to know whether damage against the monster should be considered productive (meaning the monster is not already dead or captured.)

Just out of curiosity, are you saying REFramework is doing this decryption behind the scenes when we call on getHpVital? I took a look at the "Monster has HP bar" script here and I don't see anything involving encryption here.

praydog commented 2 years ago

No, I'm talking about the underlying code the game is using for getHpVital. It looks fairly complex in the assembly code. You can view it in Cheat Engine/IDA/Ghidra using the address given to you in the Object Explorer. It should be fine to just call it once and cache it inside the monster's update function.

Alternatively, you can use a hook on via.dve.DeviceContext`1<System.Single>.read (this is the decryption function), check whether the device context == the one found in snow.enemy.EnemyPhysicalParam.getVital, and cache off the return value (the HP) there to prevent additional decryption calls other than the ones triggered by the game itself.

That is, if you need the HP. If you can find an equivalent method that does not call getHpVital internally as an implementation detail, that would probably be better and easier to reason about.

coavins commented 2 years ago

I just published v2.2.2 which reimplements some tracking of poison and blast damage, hopefully without the same performance problems. If we continue seeing crash reports, I expect we should probably post a 2.2.3 with the removal of getHpVital as described above.

coavins commented 2 years ago

I'm going to close this issue since there hasn't been any activity recently. I think the performance issues were resolved in v2.2.2.