NPBruce / valkyrie

Valkyrie GM for Fantasy Flight Board Games
Apache License 2.0
504 stars 105 forks source link

[Bug] Deactivate monster defeated button #1548

Open antontimmermans opened 2 years ago

antontimmermans commented 2 years ago

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

Smartphone

Additional context

Hobbeslionheart commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

mayjak commented 1 year ago

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

antontimmermans commented 1 year ago

Tested OK on 2.5.8a Windows build.