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

Fix Dungeon Interrupt HP bug #544

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

Conversation

ShiningDeneb
Copy link
Contributor

Since I worked on it, I wanted to submit a fix to the (old) bug of DungeonInterrupt patch where the entity HP is not synchronized with the tabulated new max HP value.

Copy link

Test Results

    14 files  ±0      14 suites  ±0   53m 8s ⏱️ -4s
10 901 tests ±0  10 900 ✅ ±0      1 💤 ±0  0 ❌ ±0 
32 904 runs  ±0  26 132 ✅ ±0  6 772 💤 ±0  0 ❌ ±0 

Results for commit d4b7897. ± Comparison against base commit e71dc51.

@Adex-8x
Copy link
Contributor

Adex-8x commented Aug 20, 2024

Unfortunately, I don’t think this is a fix. The reason this code obtains a team member’s current HP is that HP isn’t automatically healed when exiting a dungeon via dungeon interruption. With your new nop, team members’ HP will display incorrectly if they exit a dungeon via dungeon interruption with less than their maximum HP.
I’m assuming this was done with the intent to try and fix Brick-Tough having a bit of a mismatch when displaying in the overworld, so while this technically fixes that bug, it also creates a bigger one.

@irdkwia
Copy link
Contributor

irdkwia commented Aug 20, 2024

I don't approve these changes either, for one I side with what Adex already said, and because that doesn't fix other HP display (i.e. the summary in your team stats menu).
Also, the proposed changes are exactly the behaviour of the game without this patch, which doesn't care about HP in overworld because base game never need to save your current party's HP between dungeons.
That changes though with DungeonInterrupt since theoretically it introduces cases of no healing between dungeons/interruptions, in which the base game logic is not the one wanted.
So, either the logic behind Brick-Tough is changed, or the logic of the patch must be changed and drop the no healing between interruptions part, in that case the base game HP logic can be kept (though that may be implemented via parameters to let people choose between no healing/healing logics).

@ShiningDeneb
Copy link
Contributor Author

Okay I think I see the bigger issue you mention. I guess changing how Brick-Tough is accounted in the overworld sounds more preferable compared to changing the way the patch works
I will test that in the next days but I want your input on that though: do you think having Brick-Tough increase the entity current HP if it's equal to max HP does the trick?

@Adex-8x
Copy link
Contributor

Adex-8x commented Aug 23, 2024

I haven’t tried it myself, but I guess that could be an okay compromise? As well as accounting for anything else that could boost HP in the overworld, I’m not sure if Brick-Tough is the only thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants