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

[Bug] Deactivate monster defeated button #1548

Open
antontimmermans opened this issue Jan 24, 2022 · 6 comments
Open

[Bug] Deactivate monster defeated button #1548

antontimmermans opened this issue Jan 24, 2022 · 6 comments
Assignees
Labels
bug To be tested Dev is done, it should now be tested

Comments

@antontimmermans
Copy link
Collaborator

Describe the bug

When adding damage to a monster the 'defeated' button lights up when the maximum damage is reached on the monster. But if you then lower the damage to a value below its maximum health (for example because you clicked once to many on '+'), the Monster defeated button remains high-lighted.

Scenario

Any

Steps to reproduce the behavior:

  1. Go to attack monster
  2. Click on '+' to add damage to the monsters maximum --> 'defeated' button lights up
  3. Click on '-' to remove damage to below the monsters maximum
  4. See error--> the defeated button remains high-lighted (selectable)

Expected behavior

when the monster's damage is not equal; to its maximum health, the defeated button should not be high-lighted (not selectable)

LogFile

Screenshots

Valkyrie Version

v 2.5.6.

Desktop

  • OS: windows
  • Browser chrome
  • Version 11

Smartphone

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context

@Hobbeslionheart
Copy link

This occurs when the attack and evade action has been selected, mythos phase, and horror phase. The only time the defeated button actually works as intended is a user selects the monster without clicking attack or evade action.

@Hobbeslionheart
Copy link

Also if the monster's damage equal to its health and a user clicks attack or evade, the cancel button is inactive even if the hit points go below maximum. While this may seem inconsequential, we should think about sequence of how the UI is drawn in this phase.

@Hobbeslionheart
Copy link

The issue here is that the dialog/UI isn't destroyed when the HP of the monster are increased or decreased during the attack/evade, mythos or horror phase as destroyer is not called during this phase. As a result, old UIElements are left behind and appear active.

Several possible solutions:

  1. Refactor the code so that rather than redrawing the UI after every change, change only what's needed.
  2. Modifying the different phases so that they are redrawn which allows for the UI to be destroyed and made fresh similar to how it's done in the original summon.
  3. Splitting the Monster HP UI Elements by assigning a separate tag and destroying those when an element is selected.

I'm opting to do 3. 2. is also a good option, but would be significant amount of work that may be worth it as a part of a general refactor if ever done. 1. might be ideal which may speed up processing time, but would be a huge refactor.

@Hobbeslionheart
Copy link

There's actually a fourth way. Create a list of elements to store the references and delete. This was actually the easiest and least intrusive to other code bases.

Hobbeslionheart added a commit to Hobbeslionheart/valkyrie that referenced this issue May 3, 2022
Added a list to the Monster Health UI to destroy.
@mayjak mayjak self-assigned this Oct 26, 2022
@mayjak
Copy link
Collaborator

mayjak commented Oct 26, 2022

I'll keep this open until we get @Hobbeslionheart changes merged to the main codebase, but I added a workaround for now (3rd approach).

mayjak pushed a commit that referenced this issue Oct 26, 2022
mayjak pushed a commit that referenced this issue Oct 26, 2022
@mayjak mayjak added the To be tested Dev is done, it should now be tested label Oct 26, 2022
@antontimmermans
Copy link
Collaborator Author

Tested OK on 2.5.8a Windows build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug To be tested Dev is done, it should now be tested
Projects
None yet
Development

No branches or pull requests

3 participants