Continuer une attaque fait dépenser une nouvelle fois les ressources

Ulty commented 3 years ago

Si on fait une attaque qui peut être esquivée / parée / absorbée, etc, les ressources (mana, limites par combat, etc) sont depensées deux fois.

@Far2Casual : en relisant le code, je suis un peu surpris : je pensais qu'en cas de preDmg non résolu, on reprenant l'attaque là où on l'avait laissée, c'est à dire resolvePreDmgOptions. Or je vois qu'on repart de la fonction attack et qu'on recalcule tout (et en particulier on refait dépenser les ressources). Est-ce que je me suis trompé, ou ça a bien été comme ça à un moment ? Si oui, est-ce volontaire, le fait que ce ne soit plus le cas ?

Ulty commented 3 years ago

@Far2Casual, sauf si tu connais une bonne raison pour toujours tout recalculer, je vais essayer de faire reprendre à resolvePreDmgOptions. J'attends un peu de savoir ce que tu as à en dire, et je m'y colle.

Far2Casual commented 3 years ago

Réponse tardive mais ... il me semble qu'on en avait déjà discuté, et que ça créait des bugs sur certains effets ou capacités qui étaient évalués avant cette partie, et dont les conséquences et/ou les messages entraient en conflit après application de la partie "PreDmg". Par exemple, la détermination des cibles, il faut potentiellement la recalculer si des options comme "Chair à canon" sont activées.

Fondamentalement, ce que tu proposes me paraît sensé, mais je me demande ce qui est le plus simple : résoudre le problème ponctuel (la dépense des ressources) ou bien tous les effets de bord liés à ce changement. Je t'en laisse juge, mais je pense que c'est ça qui faisait qu'on avait pas été jusque là finalement.

Far2Casual commented 3 years ago

Cf. https://github.com/Ulty/COFantasy/pull/214 je pense

Far2Casual commented 3 years ago

Par contre dans https://github.com/Ulty/COFantasy/pull/284 j'ai fixé certains des effets de bord en forçant la réinitialisation de target.messages, ce qui rend peut-être le redo faisable comme tu l'entends.

Ulty commented 3 years ago

Ce n'est pas vraiment une réponse tardive. Merci en tout cas, c'est ce genre d'éléments que je cherchais. Je vais essayer d'y travailler demain.

Ulty commented 3 years ago

Réponse tardive mais ... il me semble qu'on en avait déjà discuté, et que ça créait des bugs sur certains effets ou capacités qui étaient évalués avant cette partie, et dont les conséquences et/ou les messages entraient en conflit après application de la partie "PreDmg". Par exemple, la détermination des cibles, il faut potentiellement la recalculer si des options comme "Chair à canon" sont activées.

Il suffit de choisir de ne recommencer l'attaque à zéro que pour les actions comme "Char à canon", et de recommencer à resolvePreDmgOptions pour les autres, ou bien il y a quelque chose que j'ai raté ?

Ulty commented 3 years ago

En fait, j'ai l'impression que le problème, c'est qu'un certain nombres d'actions font un redoEvent sans faire de undoEvent avant. Ou alors on n'est pas sensé faire de undoEvent, mais certaines actions le font quand même (comme la rune d'énergie ou l'esquive fatale). Du coup on n'est pas très cohérents : dans le cas où on a fait un undoEvent, on ne devrait pas mettre de options.redo, non ?

Ulty commented 3 years ago

Ça semble marcher sur quelques exemples, mais si tu peux tester aussi de ton côté, ce serait plus sûr. Je laisse l'issue ouverte le temps de voir si tout est résolu.

Far2Casual commented 3 years ago

Je teste ça ce week-end et je te dis quoi. J'ai des groupes qui utilisent beaucoup d'options de ce genre (tu prends un chevalier, un forgesort, un barde et un guerrier, c'est pas mal 😄), et je suis sur quelques gros combats que je voudrais préparer où ça va être utile, donc ça tombe bien.

Far2Casual commented 3 years ago

Bug que j'ai trouvé :

Un monstre fait une attaque de zone.
Un des joueurs utilise une Rune de Protection, puis on "continue" BUG N°1 : on dirait que le message "utilise la Rune de protection apparaît deux fois
Un des joueurs utilise ensuite une Rune d'énergie pour relancer son jet de dex BUG N°1 (suite) : cette fois par contre, le message "utilise la Rune de protection" n'apparaît qu'une fois BUG N°2 : à ce moment-là, je crois que le script "oublie" la consommation de la Rune de Protection ; l'effet de Protection est bien là, mais la rune ne semble pas utilisée. Sur l'attaque prochaine, elle est à nouveau disponible.

Far2Casual commented 3 years ago

Probablement que le bouton pour la Rune de Puissance ne devrait pas apparaître au niveau des preDmg.

Ulty commented 3 years ago

Effectivement, ce n'est pas encore au point. Je vais essayer de tout revoir, mais je ne peux pas le faire avant mardi. Si tu en as besoin avant, n'hésite pas à changer des choses.

Far2Casual commented 3 years ago

Suis désolé, j'ai pas eu le temps ce week-end. Je rapporte un autre bug, suis pas sûr que ça soit 100% lié, mais je l'avais pas avant. A l'utilisation d'Intercepter (capacité du Chevalier), il y a un souci avec le jet d'attaque :

Far2Casual commented 3 years ago

Petite contribution ici : https://github.com/Ulty/COFantasy/pull/288 Ceci devrait fixer le problème ci-dessus avec Intercepter, et le problème rencontré par Kyr avec l'échec total.

Dans les deux cas, le problème est le suivant : ces deux boutons ont pour effet de reprendre une attaque, d'en changer la cible, et d'en copier les options pour la réexécuter. Problème : dans "options" on stocke les "preDmg" et les "redo", et si ces éléments ne sont pas "nettoyés" lors de la copie des options pour relancer l'attaque, on obtient des comportements anormaux.

Typiquement dans le cas d'Intercepter, les options de PreDmg du joueur précédent s'y trouvent, ce qui fait un truc bizarre. Et pour le problème de Kyr, puisqu'on tente d'appliquer un echecTotal après un 'exemplaire' de Chevalier, l'options redo se trouvait dans l'attaque et faisait foirer le script, puisqu'en fait les cibles avaient changé.

Bref, les deux soucis illustrent probablement un défaut de conception ; dans le cas où on veut "copier" une attaque pour la réexécuter sur une autre cible, probablement que copier tout "options" n'est pas une grande idée, car on copie de cette manière des trucs qui n'ont pas trop de sens. Quelques idées :

Dans ce type de cas, on est plus précis sur les options qu'on copie, ou bien on "cleane" certaines options (c'est le fix que je t'ai push)
Peut-être qu'il faudrait séparer les options d'attaque des choses "calculées" comme preDmg et redo, qui ne devrait pas être copiées automatiquement.

Dis-moi ce que tu en penses. Je n'ai pas encore regardé

Far2Casual commented 3 years ago

Pour le reste :

Bug double message : c'est parce qu'à chaque fois que "resolvePreDmg" est appelé, le message est ajouté à la liste des messages de la cible. Comme tu reprends à cet endroit au lieu du tout début (implem précédente), cette liste n'est pas "vidée" et le message est poussé à nouveau et s'accumule. Si dans mon exemple, cinq joueurs avaient une rune de Protection et qu'ils l'activent tous, le premier aura 5x le message dans sa liste. Suggestion de fix : ne pas ajouter le message au moment où l'option est ajoutée (dans appliquerRuneDeProtection), mais uniquement où les dommages sont résolus, donc plus tard
Bug Rune de Puissance : il s'agit à mon avis d'un petit refactoring à faire dans finaliseDisplay, où il faudrait revoir l'ordre d'évaluation de tous les boutons et commencer par les preDmg, et n'afficher les autres que s'il n'y en a pas.
Bug Rune pas utilisée en cas de redo : la modification de l'attribut de la rune de protection se trouve dans persoUtiliseRuneProtection qui est appelé quand on clique sur le bouton. Le fait est que cela générait avant un évènement séparé qui ne faisait pas partie de l'attaque. En cas d'undo (par exemple une rune d'énergie plus tard comme dans l'exemple), l'évènement qui consommait la rune n'était pas undo, seule l'attaque l'était. Ce n'est plus le cas depuis le changement. Du coup l'attribut de rune est rétabli, mais pas reconsommé à la réexécution car la fonction persoUtiliseRuneProtection n'est pas appelée à nouveau, seule appliquerRuneDeProtection l'est. Deux possibilités :
1. Soit la consommation de la rune est mise dans un évènement séparé de l'attaque et n'est pas "undo" en cas de redo de l'attaque ultérieurement.
2. Soit la consommation de la rune doit être faite à chaque fois que l'attaque est faite. Attention dans ce cas à ne pas reproduire le même bug qu'avec le double message ci-dessus.

Ulty commented 3 years ago

Merci d'avoir regardé tout ça, ça va bien aider à réparer.

Ulty commented 3 years ago

@Far2Casual merci, tu m'as bien mâché le travail ;-) Il vaudrait quand même mieux re-tester pour voir si c'est ok comme ça... En particulier, je n'ai pas encore regardé assez en détail les évitements génériques (absorption au bouclier etc).

Far2Casual commented 3 years ago

Avec plaisir, je testerai ça aujourd'hui et demain.

Rien à voir, mais tu devrais vraiment ouvrir un petit Patreon. Je suis sûr que certains aimeraient te remercier pour tout l'effort mis dans ce script ;)

Far2Casual commented 3 years ago

Je suis à peu près sûr qu'on peut fermer ça, désolé.

Ulty / COFantasy

Continuer une attaque fait dépenser une nouvelle fois les ressources #286