GL-MPRI-2014 / Ocawai

OCAWAI
8 stars 3 forks source link

Animations Asynchrones #173

Closed TheoWinterhalter closed 9 years ago

TheoWinterhalter commented 9 years ago

Actuellement, les animations dans l'interface (pour les déplacements) sont asynchrones. Ce qui peut donner lieu à des incohérences lors de l'affichage (une unité est tuée avant que son assaillant n'ai terminé son déplacement).

Il serait possible de tout séquentialiser en récupérant les mises à jour une à une (et en attendant la fin de l'animation en cours avant d'en récupérer une autre). On pourrait alors plus naturellement envisager des animations pour les combats (enfin je pense qu'on n'aura pas le temps de faire grand chose).

La contrepartie est sans doute au niveau de la rapidité d'exécution…

Prenons-nous la peine de faire ce travail ?

dbusatto commented 9 years ago

Remarque sur les animations : se serait bien de les désactiver sur les unités non visible (on est obligé d'attendre la fin de trucs qu'on voit pas), donc faire un module pour bien gérer les animations, en précisant pour chaque anim si elle est en série ou en paralele, et qui ne joue que la partie visible des animations, ça serait bien

TheoWinterhalter commented 9 years ago

Oui, je suis d'accord… Mais avant tout, est-ce qu'on veut vraiment les avoir en parallèle ?

Je pense qu'il est raisonnable de dire que l'on pop les updates au fur et à mesure et que le joueur ne récupère pas la main (son tour de jeu) avant la fin des animations.

VLanvin commented 9 years ago

Je ne suis pas sur qu'on veuille avoir des anim en parallèle. Par contre, actuellement toutes les updates se font dans le module Render (ce qui est assez sale non ?), du coup, il serait peut-être bien de créer une classe Updater qui se chargerait, à chaque frame, de gérer la pile d'updates (et pourquoi pas passer la pile dans Updater plutôt que ClientData). Ainsi, on pourrait avoir par exemple un attribut booléen dans la classe Updater qui indiquerait lorsqu'une anim est en cours, et qui bloque la récupération de nouvelles updates tant que l'anim n'est pas terminée.

Quant au problème des unités qui se déplacent uniquement lorsqu'on les découvre, je crois (je n'ai pas le code sous les yeux) que c'est parce qu'on fait l'update des anims uniquement lors de l'affichage d'une unité. En fait, il faudrait updater les animations à chaque frame y compris si on n'affiche pas l'unité, ce dont le module Updater se chargerait parfaitement.

Note : Je ne me suis pas trop embêté pour l'affichage, j'affiche uniquement les unités visibles en utilisant get_visible_army_for. Ca pose problème car ça ne prend pas en compte la position d'une unité en cours de déplacement (donc si l'unité sort et rentre immédiatement dans le brouillard, on ne la verra pas du tout). Il faudra changer ça.

TheoWinterhalter commented 9 years ago

Je pense que la création d'un tel module (Update, Animation ?) s'impose donc. Et on n'a pas forcément besoin d'un booléen à vrai dire… Si on a une option sur l'animation en cours, ce n'est pas nécessaire.

TheoWinterhalter commented 9 years ago

J'ai push dans la branche update un nouvel objet pour gérer les animations et les modifications qui vont avec.

De cette manière, le son est enfin séparé du module de rendu ! En revanche, je me retrouve confronté à un problème du à la mise en série des animations; les players, eux, sont mis à jour indépendamment, et donc si le moteur de mise à jour ne gèle pas leur déplacement, ils sont d'abord téléportés à leur position finale, et seulement ensuite, quand c'est leur tour, l'animation démarre en les faisant partir de leur position de départ.

C'est pour cette raison que je l'ai mis dans une branche à part, pour l'instant on a l'impression que c'est cassé.

EDIT : La bonne nouvelle c'est que les ennemis qui sortent du brouillard de guerre ne subissent plus leur animation à ce moment. Mais ils la subissent de manière cachée…

TheoWinterhalter commented 9 years ago

Maintenant, la caméra cible les actions qui sont gérées. Evidemment, ça va souvent aller dans le brouillard (ça nous en rajoute du boulot tout ça :p).

TheoWinterhalter commented 9 years ago

Bon, j'ai corrigé (du moins temporairement) le problème des unités qui se téléportent. (Mais, en revanche, la minimap est en avance).

J'ai encore un petit souci de Not_found qui arrive de temps en temps (j'ai pas très bien compris ce qui le cause). Et les animations ne prennent toujours pas en compte le brouillard, ce qui est gênant.

TheoWinterhalter commented 9 years ago

Spam update (il faut désactiver les mails de github aussi !).

Maintenant, ça commence déjà plus à ressembler à quelque chose.

Il reste toujours un problème d'inconsistence visuelle.

EDIT : Si quelqu'un peut m'aider à couper le robinet des mises à jour pour que je ne sois pas obligé de faire une copie de tout dans l'interface, ce serait coule. Merci !

TheoWinterhalter commented 9 years ago

Alors, j'ai un peu regardé le code de Game_engine et je vois à peu près ce qui ne va pas pour moi.

J'ai l'impression que le dernier point va m'obliger à effectuer une copie des unités pendant les tours adverses. J'ai pas vraiment envie de faire ça, c'est un peu sale non ? Quelqu'un a des idées ?

(Joyeux Noël)

VLanvin commented 9 years ago

Il faudrait que le module Game_engine ne modifie pas les unités en place (sauf si on est en réseau, dans ce cas il ne met à jour que les unités côté serveur, et passe l'info au dealer côté client). Ainsi, du côté client/en local, c'est l'interface qui se charge de mettre à jour les unités (donc quand l'anim est terminée). Et finalement, ça me parraît assez logique comme ça.

TheoWinterhalter commented 9 years ago

Oui, je suis plutôt d'accord. Et c'est plus extensible au réseau. D'autant plus que tout est passé par des ID déjà, donc pas besoin de manipuler les mêmes objets physiques. Je ne réalise pas trop, est-ce une grosse modification ? (Du côté moteur, car bien sûr du côté client, il faut récupérer les updates et mettre à jour en conséquence, avec mon module Update qui commande quand on a le droit de pop).

TheoWinterhalter commented 9 years ago

Pour l'instant, tout passe par les players, et ce n'est pas la méthode update qui fait la modification. Il faudrait donc avoir une copie des players dans Game. Mais comme on dialogue avec le moteur uniquement via les players…

On pourrait peut-être commencer par faire une copie des autres players que le ClientPlayer après avoir appelé game_engine#init (et une copie qui va au fond des choses, ce qu'on veut surtout, c'est avoir une copie des unités et bâtiments), et ajouter la liste des autres players à ClientPlayer. Lorsqu'un pop_update est appelé, on met à jour les players stockés en conséquence.

TheoWinterhalter commented 9 years ago

J'ai essayé de faire ça dans une branche copycat, mais c'est peu encourageant. Je crois que j'ai coupé certaines mises à jour du moteur lui même, mais pas toutes…

TheoWinterhalter commented 9 years ago

Je continue mon monologue. Ça se précise un peu. J'ai réussi à séparer (à priori) les players possédés par le moteur et par l'interface (à part pour le brouillard de guerre ?). J'ai ajouté au module Updates la gestion de ces mises à jour (elles ne sont plus que graphiques). Ça reste à terminer.

Par contre, quelques trucs sont cassés à cause de ça (notamment l'information sur la case, data#player_of lance assert false sur une unité qu'il m'a renvoyé. Je ne comprends pas bien comment c'est possible.

TheoWinterhalter commented 9 years ago

J'ai vraiment plein de problèmes avec ces modifications ! J'ai encore des bugs débiles que je ne comprends pas… A priori, maintenant on peut s'attaquer soi-même…

A part ça, le module est plutôt en phase terminale. Il faut débogguer pour trouver d'où viennent ces foutus problèmes. Est-ce que ça tente quelqu'un de m'expliquer ce que je fais de travers ? (Normalement, la fonction ack_update seule est à regarder).

VLanvin commented 9 years ago

Je pense (je n'ai pas regardé le fonctionnement en détail, c'est donc juste un premier coup d'oeil) que le problème vient des Oo.copy. Je pense que lorsqu'on récupère l'unité sous le curseur, on récupère l'originale, mais on tente de la comparer à sa copie dans l'armée du joueur, ce qui renverra faux (ou inversement). Elle sera donc déclarée non-alliée et pourra être attaquée.

PS : Je sais que ça ne vient pas de toi, mais c'est quoi cette update "Classement" ?!?

TheoWinterhalter commented 9 years ago

Donc, les problèmes maintenant seraient localisés dans Game. C'est possible en effet. (Il doit y avoir un problème par là puisqu'il est également impossible de construire des unités également).

Pour ce qui est de l'update Classement, c'est vraiment bizarre. Elle n'a pas d'argument donc je ne vois pas vraiment l'intérêt…

TheoWinterhalter commented 9 years ago

Une solution consisterait donc à passer aux comparaisons d'id pour tous ces objets (players, bâtiments, unités) plutôt que l'égalité classique. Ou identifier l'endroit où il y a une inconsistance (plus propre et mieux sur le long terme, même si on peut le coupler avec la première solution) copie/pas copie.

J'ai testé pour la construction de bâtiments, si on utilise la comparaison d'id ça fonctionne…

VLanvin commented 9 years ago

J'ai fixé l'auto-attaque en comparant sur les id dans Logics (fonction units_in_range). Mais je n'arrive quand même pas à comprendre pourquoi le test d'égalité renvoyait faux, je ne vois pas où les joueurs auraient pu être copiés.

EDIT : Note qu'on a perdu le fait que les unités jouées soient grisées. Ca doit venir du même problème.

TheoWinterhalter commented 9 years ago

Parfois certaines unités d'un même joueur se gênent pour le déplacement. Elles ne peuvent plus s'attaquer, mais ça reste ennuyeux.

VLanvin commented 9 years ago

Il va falloir aller chercher tous les endroits où on a fait une comparaison simple et changer par un test sur les ids :)

TheoWinterhalter commented 9 years ago

Ce qui m'échappe, c'est pourquoi des successions comme celles-ci ne fonctionnent pas :

let old_b = Logics.building_of_id b#get_id data#players data#neutral_buildings in
let player = Logics.find_player pid data#players in
player#delete_building old_b#get_id

La troisième ligne me renvoie un Not_found, je trouve ça curieux.

EDIT : Je précise que je suis dans un

match old_b#player_id with Some pid

à partir de la deuxième ligne.

VLanvin commented 9 years ago

Au passage, sur un sujet un peu différent, il faudrait quand même que j'implémente une meilleure gestion des threads (avec un mutex). Du coup, je me demandais, il y a moyen de savoir quand une animation est terminée ? Je pense que ça serait pas mal de bloquer/débloquer le mutex au début/fin de l'anim.

Pour ton message : déjà je trouve pas terrible que le Not_found ne soit pas catché par Player... Mais à part ça, si tu remplaces old_b#get_id par b#get_id dans la dernière ligne, ça ne marche pas non plus ?

EDIT -- D'ailleurs, tu es sûr que old_b appartient bien à player ?

EDIT2 -- Peut-être qu'il serait bon aussi de faire des Hashtbl.replace plutôt que Hashtbl.add dans le module Player, dans add_unit/building.

TheoWinterhalter commented 9 years ago

Tu as la méthode update qui est celle appelée à l'extérieur, elle vérifie si il y a une animation en cours (qui peut être une pause d'un certain nombre de frames, ça peut se changer pour devenir un temps de sleep) et sinon, on lit d'éventuelles nouvelles updates. Sinon, on peut transformer l'utilisation de la variable d'instance avec des accesseurs internes qui font quelque chose lors d'un Rien -> Autre ou Autre -> Rien.


Non, ça ne change rien. En même temps c'est normal puisque d'après la fonction de recherche,

b#get_id = old_b#get_id
TheoWinterhalter commented 9 years ago

Et bien normalement, la mise à jour n'est pas effectuée, donc old_b correspond à ce que j'ai en mémoire (b sera sa future version). Je récupère l'id de son propriétaire (s'il en a un) et dans ce cas, je supprime b de sa liste.

Si je me fais capturer un bâtiment avec cette ligne commentée, j'ai une superposition de deux bâtiments. Si je la décommente, j'ai cette erreur dès le tour du joueur adverse. Est-ce qu'il y aurait un doublon, mais pas pour le actual_player ?


Remplacer les adds me semble une bonne idée.

TheoWinterhalter commented 9 years ago

On dirait aussi qu'il y a une double perte des PV parfois… L'unité perd des PV avant l'animation, puis juste après. C'est assez gênant.

TheoWinterhalter commented 9 years ago

J'ai du rajouter la comparaison par ID aussi dans Game pour pouvoir attaquer sans se déplacer. On en déduit que cdata#unit_at_position cursor#position et cdata#player_unit_at_position cursor#position cdata#actual_player sur une position qui nous appartient ne renvoient pas la même unité, mais deux copies de la même.

EDIT : On semble apprendre que cdata#actual_player ne renvoie pas un player de la liste.

TheoWinterhalter commented 9 years ago

J'ai fait une copie plus propre qui s'hérite sans problème. J'ai aussi fait en sorte que le client_data possède deux players identiques pour actual_player et son équivalent dans la liste de tous les joueurs.

Enfin, je conserve uniquement une copie de cet actual_player (avant il était encore partagé avec le moteur je crois). Malheureusement, de ce fait, cdata#actual_player#event_state n'est pas mis à jour et il n'est plus possible de déplacer les unités… (J'en ai marre des ces bugs à la chaîne).


Edit : Il s'agit de get_next_action qui n'est plus appelé à cause de la séparation moteur/interface… Une idée de quoi faire @VLanvin ?

TheoWinterhalter commented 9 years ago

Je rajoute quelques mises à jours depuis le moteur parce que sinon certaines opérations ne sont plus faîtes que sur le moteur. Je vous annonce donc le retour du marquage des unités jouées ainsi que celui de l'argent gagné.

En revanche, je me suis fait jeter par le moteur en voulant construire un hélicopter alors que j'avais 100 fleurs. Il reste donc du boulot pour la synchronisation.

TheoWinterhalter commented 9 years ago

J'ai corrigé l'affichage des unités adverses qui étaient initialement sous le brouillard de guerre. Il semblerait que la méthode data#visible_unit_at_position ne donne pas le bon résultat… (Le brouillard a toujours un comportement chaotique donc je ne sais pas trop d'où ça vient).

TheoWinterhalter commented 9 years ago

J'ai l'impression qu'on commence à en voir le bout. Avec un peu de chance ça pourra aller dans master d'ici la deadline.

Petit récapitulatif :

Il va falloir faire de nouveau un tour du propriétaire pour vérifier que tout fonctionne correctement (aux bogues de master près bien entendu).

J'apprécierais des retours (surtout que là je suis content donc il faut me briser le moral :)).

PLBegay commented 9 years ago

Catégories réécriture catégories réécriture catégories réécriture catégories réécriture catégories réécriture catégories réécriture catégories réécriture catégories réécriture.

De rien. Le 3 janv. 2015 18:29, "Théo Winterhalter" notifications@github.com a écrit :

J'ai l'impression qu'on commence à en voir le bout. Avec un peu de chance ça pourra aller dans master d'ici la deadline.

Petit récapitulatif :

-

Les PV sont mieux gérés (en fait j'avais mal compris ce que représentait un paramètre, j'ai du aller lire le code);

On peut de nouveau créer des unités sans que le moteur nous fasse passer le tour, l'argent est mis à jour et dépensé correctement;

J'ai touché à beaucoup de fichiers pour changer le type Action.t de sorte qu'il n'utilise plus que des identifiants;

Le moteur envoie aussi la mise à jour concernant l'attaque si une unité est tuée;

Enfin, j'ai décidé que le moteur gèrerait la destruction et l'ajout des bâtiments lui-même (après tout, il y avait des mise à jours prévues à cet effet mais elles n'étaient pas utilisées) et ça a pour effet de corriger (enfin !) le bug associé aux bâtiments.

Il va falloir faire de nouveau un tour du propriétaire pour vérifier que tout fonctionne correctement (aux bogues de master près bien entendu).

J'apprécierai des retours (surtout que là je suis content donc il faut me briser le moral :)).

— Reply to this email directly or view it on GitHub https://github.com/GL-MPRI-2014/GL_MPRI_2014/issues/173#issuecomment-68602407 .

VLanvin commented 9 years ago

Pas bien de spammer les tickets, surtout que la deadline du GL est plus proche que celle de catégories, et encore plus que celle de réécriture :)

@iSheeft, j'ai fait une partie rapide en cherchant les bugs, j'en ai pas trouvé. Donc pour l'instant tout va bien !

TheoWinterhalter commented 9 years ago

Yay ! Merci @VLanvin. Je ferme le ticket (au moins temporairement) et je balance une pull request.