Skip to content

feat: add death system#30

Merged
AyakorK merged 4 commits intodevelopfrom
feature/death_scene
Oct 25, 2024
Merged

feat: add death system#30
AyakorK merged 4 commits intodevelopfrom
feature/death_scene

Conversation

@Thomlam
Copy link
Copy Markdown
Contributor

@Thomlam Thomlam commented Oct 25, 2024

Summary by CodeRabbit

  • Nouvelles Fonctionnalités

    • Introduction de nouveaux éléments d'interface utilisateur comme LifeCanvas, DialogueSystem, et DeathPanel.
    • Ajout de la gestion de l'état de mort du joueur avec un nouvel affichage UI.
    • Implémentation de la méthode ReturnToMenu pour retourner au menu principal.
  • Corrections de Bugs

    • Amélioration de la mise à jour de la barre de santé lors des ajustements de vie.
  • Refactorisation

    • Restructuration complète du prefab Main Camera, renommé en Chrono, et mise à jour de sa hiérarchie.
    • Suppression de la classe GameOverManager, sans logique fonctionnelle.
  • Changements dans la Scène

    • Réorganisation de la scène EmptyRoom_2 avec ajout et suppression de plusieurs objets.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 25, 2024

Warning

Rate limit exceeded

@Thomlam has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 35 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Files that changed from the base of the PR and between 581a32d and 343e933.

Walkthrough

Les modifications apportées dans cette demande de tirage concernent principalement la réorganisation et la mise à jour du prefab de la caméra principale dans Unity, renommé en "Chrono". Des objets de jeu supplémentaires ont été ajoutés pour gérer l'interface utilisateur, tandis que plusieurs composants ont été supprimés, notamment le composant de caméra. Des changements ont également été effectués dans la scène EmptyRoom_2, avec la suppression de certains objets et l'ajout de nouveaux. Enfin, la classe GameOverManager a été complètement supprimée, et des améliorations ont été apportées à la gestion de la santé du joueur.

Changes

Fichier Résumé des changements
Assets/Prefabs/Main Camera.prefab Renommage de "Main Camera" à "Chrono", ajout de nouveaux objets de jeu (LifeCanvas, DialogueSystem, etc.), suppression de composants tels que Camera et AudioListener.
Assets/Scenes/EmptyRoom_2.unity Suppression d'objets de jeu (Global Light 2D, Stage, Chrono, DialogueSystem), ajout de nouveaux objets (EventSystem, PostProcessing, etc.).
Assets/Scripts/UI/GameOverManager.cs Suppression complète de la classe GameOverManager, y compris les méthodes Start() et Update().
Assets/Scripts/UI/GameOverManager.cs.meta Suppression du fichier de métadonnées associé à GameOverManager.cs.
Assets/Scripts/UI/PlayerHealth.cs Ajout de la gestion du deadCanvas, mise à jour des méthodes TakeDamage, Heal, et ajout de ReturnToMenu().
Assets/Scripts/Map Control/DoorSystem.cs Modification de la visibilité de la méthode SocketEmitter dans la classe RoomManager.

Possibly related PRs

  • feat: Add Socket to props to send them to back-end #8 : Les changements dans Assets/Scenes/EmptyRoom_2.unity incluent l'ajout de l'objet de jeu DialogueSystem, qui est directement lié aux modifications dans le PR principal qui a également introduit un objet DialogueSystem dans le prefab de la caméra principale.
  • Feature/add traps #19 : La classe Bear_Trap ajoutée dans Assets/Scripts/Trap/Bear_Trap/Bear_Trap.cs est pertinente car elle introduit une nouvelle fonctionnalité de piège, en lien avec la restructuration des éléments d'interface utilisateur et des objets de jeu dans le PR principal, notamment avec l'ajout de DeathPanel et d'autres éléments d'interface utilisateur.
  • feat: Link the chat to the back sockets #25 : Les modifications apportées au Dialogue_vertical.prefab et l'introduction du DialogueSystem dans le contexte de liaison de la fonctionnalité de chat au backend sont pertinentes car elles se connectent aux changements d'interface utilisateur effectués dans le PR principal, qui se concentre également sur les éléments d'interface utilisateur et leur agencement.
  • feat: Link chat to backend #26 : Semblable au feat: Link the chat to the back sockets #25, ce PR se concentre sur la liaison du chat au backend, ce qui est pertinent par rapport aux changements d'interface utilisateur dans le PR principal impliquant le DialogueSystem et d'autres composants d'interface utilisateur.

Suggested reviewers

  • AyakorK

Poem

Dans le jardin des pixels, je danse et je joue,
Chrono veille, avec un regard si doux.
Des canapés de vie, des dialogues en fleurs,
La santé du joueur, un trésor de bonheurs.
O, joie des changements, dans ce monde en couleurs! 🐰✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Thomlam Thomlam requested a review from AyakorK October 25, 2024 09:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Outside diff range and nitpick comments (1)
Assets/Scripts/UI/PlayerHealth.cs (1)

4-5: Vérification des dépendances requises pour le système de mort

Les nouvelles importations et le champ deadCanvas sont cohérents avec l'implémentation du système de mort. Cependant, il serait préférable de documenter le rôle du canvas de mort dans l'interface utilisateur.

Suggestion d'ajout de documentation :

+    [Tooltip("Canvas affiché lors de la mort du joueur")]
     public GameObject deadCanvas;

Also applies to: 16-16

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 94f0ea3 and d940dd1.

📒 Files selected for processing (5)
  • Assets/Prefabs/Main Camera.prefab (4 hunks)
  • Assets/Scenes/EmptyRoom_2.unity (7 hunks)
  • Assets/Scripts/UI/GameOverManager.cs (0 hunks)
  • Assets/Scripts/UI/GameOverManager.cs.meta (0 hunks)
  • Assets/Scripts/UI/PlayerHealth.cs (4 hunks)
💤 Files with no reviewable changes (2)
  • Assets/Scripts/UI/GameOverManager.cs
  • Assets/Scripts/UI/GameOverManager.cs.meta
🔇 Additional comments (5)
Assets/Scripts/UI/PlayerHealth.cs (1)

86-91: 🛠️ Refactor suggestion

Sécurisation du retour au menu

La méthode de retour au menu présente des risques potentiels.

  1. Vérifier que la scène "Menu" existe bien
  2. Gérer les erreurs potentielles lors de la destruction de l'objet temporaire
     public void ReturnToMenu()
     {
-        GameObject socket = GameObject.FindWithTag("Temporary");
-        if (socket != null) Destroy(socket.gameObject);
-        SceneManager.LoadScene("Menu");
+        try
+        {
+            GameObject socket = GameObject.FindWithTag("Temporary");
+            if (socket != null)
+            {
+                Destroy(socket.gameObject);
+            }
+            
+            if (SceneUtility.GetBuildIndexByScenePath("Menu") != -1)
+            {
+                SceneManager.LoadScene("Menu");
+            }
+            else
+            {
+                Debug.LogError("La scène 'Menu' n'existe pas dans le build !");
+            }
+        }
+        catch (System.Exception e)
+        {
+            Debug.LogError($"Erreur lors du retour au menu : {e.Message}");
+        }
     }
✅ Verification successful

La scène Menu est correctement configurée, mais la gestion d'erreurs peut être améliorée

La vérification a confirmé que :

  • La scène "Menu" est bien incluse dans les paramètres de build Unity
  • Le chemin exact est Assets/Scenes/UI/Menu.unity
  • Cette fonctionnalité est également utilisée dans PauseMenu.cs de manière similaire

La suggestion de refactoring proposée reste pertinente pour une meilleure robustesse du code, mais le risque initial concernant l'existence de la scène est écarté.

Points d'amélioration recommandés :

  • Ajouter un try-catch pour la gestion des erreurs potentielles
  • Harmoniser la gestion du retour au menu entre PlayerHealth.cs et PauseMenu.cs
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Vérifier l'existence de la scène Menu dans le build
rg -l "Menu" "ProjectSettings/EditorBuildSettings.asset"

Length of output: 96


Script:

#!/bin/bash
# Vérifier le contenu du fichier de build settings pour confirmer que la scène Menu est bien incluse
cat ProjectSettings/EditorBuildSettings.asset

Length of output: 500


Script:

#!/bin/bash
# Rechercher d'autres références à la scène Menu dans le projet
rg -g "!{*.meta,*.asset}" "Menu" -A 2 -B 2

Length of output: 6815


Script:

#!/bin/bash
# Vérifier si d'autres scripts utilisent également LoadScene avec "Menu"
ast-grep --pattern 'SceneManager.LoadScene("Menu")'

Length of output: 204

Assets/Scenes/EmptyRoom_2.unity (3)

407-410: Bonne implémentation du système de mort !

L'ajout de la référence deadCanvas au prefab du joueur permet une bonne gestion de l'interface utilisateur lors de la mort du personnage.


227-227: Configuration correcte de l'état initial du mini-jeu

La désactivation par défaut du mini-jeu Pierre-Papier-Ciseaux est une bonne pratique.


702-758: Vérifier le placement de l'arbalète

L'arbalète est placée assez haut dans la scène (y = 8.66). Assurez-vous que cette position est intentionnelle et qu'elle s'intègre bien avec le système de mort.

✅ Verification successful

Placement de l'arbalète validé dans le contexte des pièges

La position élevée de l'arbalète est cohérente avec son rôle de piège, comme le confirme son emplacement dans le dossier Assets/Prefabs/Traps/Crossbow/. La présence de variantes directionnelles (up, down, side_left, side_right) indique un système de pièges bien conçu avec des arbalètes placées à différentes positions stratégiques.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Vérifier les autres objets à cette hauteur
rg "m_LocalPosition.y.*8\." --type yaml

Length of output: 190


Script:

#!/bin/bash
# Search for y position values in Unity scene files
fd -e unity -x rg -l "m_LocalPosition.y"

# Then get specific y values around 8-9 range
fd -e unity -x rg "m_LocalPosition.y.*value:.*[89]"

Length of output: 473


Script:

#!/bin/bash
# Let's check EmptyRoom_2.unity for all y positions to understand the vertical layout
rg "m_LocalPosition.y.*value:" ./Assets/Scenes/EmptyRoom_2.unity -A 1

# Also check for any prefabs with "crossbow" in their name
fd "crossbow.*prefab"

Length of output: 734

Assets/Prefabs/Main Camera.prefab (1)

1472-1478: Confirmer l'inactivation du GameObject 'DeadCanvas'

Le GameObject 'DeadCanvas' est désactivé (m_IsActive: 0) aux lignes 1472-1478. Vérifiez si cela est intentionnel. Si 'DeadCanvas' est censé gérer l'affichage lors de la mort du joueur, son inactivation pourrait empêcher son fonctionnement.

Comment on lines +32 to +37
currentHealth = 0;
}
UpdateHealthBar();
if (currentHealth <= 0)
{
Die();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Amélioration de la gestion de la mort du joueur

La logique de vérification de la mort est correcte, mais il manque un retour visuel ou sonore pour indiquer les dégâts subis.

         currentHealth -= amount;
         if (currentHealth < 0)
         {
             currentHealth = 0;
         }
         UpdateHealthBar();
+        // Feedback visuel des dégâts
+        StartCoroutine(DamageFlash());
         if (currentHealth <= 0)
         {
             Die();
         }

Committable suggestion was skipped due to low confidence.

Comment on lines +51 to +58
void Die()
{
if (deadCanvas != null)
{
deadCanvas.SetActive(true);
}
GetComponent<PlayerMovement>().enabled = false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Vérification de la méthode de mort

La désactivation du mouvement du joueur est implémentée, mais il manque plusieurs aspects importants pour une expérience de mort complète.

Suggestions d'améliorations :

  1. Ajouter un délai avant l'affichage du canvas de mort
  2. Désactiver les collisions du joueur
  3. Jouer une animation de mort
  4. Sauvegarder les statistiques de la partie
     void Die()
     {
+        // Désactiver les collisions
+        GetComponent<Collider2D>().enabled = false;
+        
+        // Jouer l'animation de mort
+        StartCoroutine(PlayDeathSequence());
+        
         if (deadCanvas != null)
         {
-            deadCanvas.SetActive(true);
+            StartCoroutine(ShowDeathScreen());
         }
         GetComponent<PlayerMovement>().enabled = false;
     }

Committable suggestion was skipped due to low confidence.

Comment on lines +20 to +23
currentHealth = maxHealth;
UpdateHealthBar();

if (deadCanvas != null) deadCanvas.SetActive(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Initialisation sécurisée du canvas de mort

L'initialisation est correcte, mais la vérification de nullité pourrait être plus explicite.

-        if (deadCanvas != null) deadCanvas.SetActive(false);
+        if (deadCanvas != null)
+        {
+            deadCanvas.SetActive(false);
+        }
+        else
+        {
+            Debug.LogWarning("DeadCanvas n'est pas assigné dans l'inspecteur !");
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
currentHealth = maxHealth;
UpdateHealthBar();
if (deadCanvas != null) deadCanvas.SetActive(false);
currentHealth = maxHealth;
UpdateHealthBar();
if (deadCanvas != null)
{
deadCanvas.SetActive(false);
}
else
{
Debug.LogWarning("DeadCanvas n'est pas assigné dans l'inspecteur !");
}

Comment on lines +924 to +974
Camera:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 4190906850344886629}
m_Enabled: 1
serializedVersion: 2
m_ClearFlags: 2
m_BackGroundColor: {r: 0.061365236, g: 0.06292273, b: 0.066037714, a: 0}
m_projectionMatrixMode: 1
m_GateFitMode: 2
m_FOVAxisMode: 1
m_Iso: 200
m_ShutterSpeed: 0.005
m_Aperture: 16
m_FocusDistance: 10
m_FocalLength: 50
m_BladeCount: 5
m_Curvature: {x: 2, y: 11}
m_BarrelClipping: 0.25
m_Anamorphism: 0
m_SensorSize: {x: 36, y: 24}
m_LensShift: {x: 0, y: 0}
m_NormalizedViewPortRect:
serializedVersion: 2
x: 0
y: 0
width: 1
height: 1
near clip plane: 0.2
far clip plane: 10
field of view: 53.400234
orthographic: 0
orthographic size: 5.056125
m_Depth: 0
m_CullingMask:
serializedVersion: 2
m_Bits: 87
m_RenderingPath: -1
m_TargetTexture: {fileID: 0}
m_TargetDisplay: 0
m_TargetEye: 0
m_HDR: 1
m_AllowMSAA: 0
m_AllowDynamicResolution: 0
m_ForceIntoRT: 0
m_OcclusionCulling: 0
m_StereoConvergence: 10
m_StereoSeparation: 0.022
--- !u!81 &4273087511613948295
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Déplacer le composant 'Camera' de 'Main Camera' vers 'Chrono'

Le composant 'Camera' est actuellement attaché au GameObject 'Main Camera' (lignes 924-974). Étant donné que 'Chrono' semble remplacer 'Main Camera', il serait préférable de déplacer le composant 'Camera' vers 'Chrono' pour maintenir les fonctionnalités de la caméra dans votre scène.

Pour ce faire :

  • Supprimez le composant 'Camera' de 'Main Camera'.
  • Ajoutez un composant 'Camera' à 'Chrono' avec les mêmes paramètres que précédemment.

Comment on lines +975 to +982
AudioListener:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 4190906850344886629}
m_Enabled: 1
--- !u!114 &8154041069363273802
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Déplacer le composant 'AudioListener' vers 'Chrono'

Le composant 'AudioListener' est toujours présent sur 'Main Camera' (lignes 975-982). Pour assurer une gestion audio correcte, envisagez de déplacer 'AudioListener' vers 'Chrono', surtout si 'Main Camera' est destiné à être supprimé.

Comment on lines +583 to +588
m_TagString: Untagged
m_Icon: {fileID: 0}
m_NavMeshLayer: 0
m_StaticEditorFlags: 2147483647
m_StaticEditorFlags: 0
m_IsActive: 1
--- !u!4 &588703733079249260
Transform:
--- !u!224 &7595830232742396208
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Attention à la duplication du composant Chrono

Le composant 'Chrono' est présent à la fois dans le prefab 'Main Camera' et dans 'LifeCanvas'. Pour éviter toute confusion ou conflit potentiel, il est recommandé de :

  • Supprimer le composant Chrono du prefab 'Main Camera' puisqu'il est déjà géré par le prefab 'LifeCanvas'
  • Mettre à jour toutes les références pour pointer vers le Chrono du LifeCanvas
🔗 Analysis chain

Vérifier la présence simultanée de 'Main Camera' et 'Chrono' dans le prefab

Il semble que le GameObject 'Main Camera' (lignes 901-907) existe toujours dans le prefab en plus de 'Chrono' (lignes 583-588). Pour éviter des conflits ou des comportements inattendus, assurez-vous que 'Main Camera' n'est plus nécessaire et envisagez de le supprimer si c'est le cas.

Pour vérifier s'il reste des références à 'Main Camera' dans le projet, vous pouvez exécuter le script suivant :


Let me generate additional scripts to check for any dependencies or relationships between 'Main Camera' and 'Chrono' in the prefab:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description : Recherche des références à 'Main Camera' dans le projet.

# Test : Rechercher les occurrences de 'Main Camera' dans les fichiers de scène et de script.
rg 'Main Camera' -t yaml -g '*.unity' -g '*.prefab'
rg 'Main Camera' -t cs -g '*.cs'

Length of output: 3834


Script:

#!/bin/bash
# Vérifier le contenu du prefab Main Camera pour comprendre la relation avec Chrono
cat "Assets/Prefabs/Main Camera.prefab"

# Rechercher des références spécifiques à Chrono
rg "Chrono" -t yaml -g "*.prefab" -g "*.unity"

Length of output: 61019

@Thomlam Thomlam self-assigned this Oct 25, 2024
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (2)
Assets/Scripts/Map Control/DoorSystem.cs (1)

Line range hint 26-64: Attention à la gestion des ressources dans OnTriggerEnter2D

La méthode est assez longue et gère plusieurs responsabilités. Considérez de :

  1. Extraire la logique de nettoyage des pièges dans une méthode séparée
  2. Ajouter des commentaires pour expliquer la logique du corridor X7
+    private void CleanupTraps()
+    {
+        GameObject[] traps = GameObject.FindGameObjectsWithTag("trap");
+        if (traps == null)
+        {
+            Debug.Log("Aucun piège trouvé");
+            return;
+        }
+        foreach (GameObject trap in traps)
+        {
+            Destroy(trap);
+        }
+    }
Assets/Scripts/UI/PlayerHealth.cs (1)

17-19: Ajoutez une documentation pour les champs importants

Les nouveaux champs clientSocket et deadCanvas manquent de documentation expliquant leur rôle dans le système de mort.

+    /// <summary>
+    /// Socket client pour la communication réseau lors de la mort du joueur
+    /// </summary>
     private SocketIOUnity clientSocket;

+    /// <summary>
+    /// Canvas affiché lors de la mort du joueur
+    /// </summary>
     public GameObject deadCanvas;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d940dd1 and 581a32d.

📒 Files selected for processing (2)
  • Assets/Scripts/Map Control/DoorSystem.cs (1 hunks)
  • Assets/Scripts/UI/PlayerHealth.cs (4 hunks)
🔇 Additional comments (2)
Assets/Scripts/Map Control/DoorSystem.cs (2)

68-71: Bon changement de visibilité pour SocketEmitter !

La modification de la visibilité de la méthode en private est appropriée car elle n'est utilisée qu'en interne dans la classe.


Line range hint 16-24: Suggestion : Vérifier la connexion socket au démarrage

Il serait judicieux de vérifier si le socket est correctement initialisé dans la méthode Start.

Comment on lines +68 to 71
private async void SocketEmitter()
{
await clientSocket.EmitAsync("rooms:corridor");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Suggestion : Ajouter une gestion d'erreur pour l'émission de socket

Il serait prudent d'ajouter une gestion des exceptions pour l'appel socket asynchrone.

-    private async void SocketEmitter()
-    {
-        await clientSocket.EmitAsync("rooms:corridor");
-    }
+    private async void SocketEmitter()
+    {
+        try {
+            await clientSocket.EmitAsync("rooms:corridor");
+        } catch (System.Exception e) {
+            Debug.LogError($"Erreur lors de l'émission du socket : {e.Message}");
+        }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private async void SocketEmitter()
{
await clientSocket.EmitAsync("rooms:corridor");
}
private async void SocketEmitter()
{
try {
await clientSocket.EmitAsync("rooms:corridor");
} catch (System.Exception e) {
Debug.LogError($"Erreur lors de l'émission du socket : {e.Message}");
}
}

Comment on lines +60 to +68
void Die()
{
SocketEmitter();
if (deadCanvas != null)
{
deadCanvas.SetActive(true);
}
GetComponent<PlayerMovement>().enabled = false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Améliorer le retour utilisateur lors de la mort

La méthode Die pourrait bénéficier d'effets visuels et sonores supplémentaires.

     void Die()
     {
+        // Effet visuel de mort
+        StartCoroutine(DeathEffect());
+        
         SocketEmitter();
         if (deadCanvas != null)
         {
-            deadCanvas.SetActive(true);
+            // Animation de fade-in du canvas
+            StartCoroutine(FadeInDeathCanvas());
         }
         GetComponent<PlayerMovement>().enabled = false;
+        // Désactiver les autres composants du joueur
+        GetComponent<Collider2D>().enabled = false;
+        // Jouer un son de mort
+        AudioSource.PlayClipAtPoint(deathSound, transform.position);
     }

+    private IEnumerator DeathEffect()
+    {
+        // Effet de flash rouge
+        SpriteRenderer renderer = GetComponent<SpriteRenderer>();
+        Color originalColor = renderer.color;
+        renderer.color = Color.red;
+        yield return new WaitForSeconds(0.1f);
+        renderer.color = originalColor;
+    }

Committable suggestion was skipped due to low confidence.

Comment on lines +96 to +101
public void ReturnToMenu()
{
GameObject socket = GameObject.FindWithTag("Temporary");
if (socket != null) Destroy(socket.gameObject);
SceneManager.LoadScene("Menu");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Sécuriser la transition vers le menu

La méthode ReturnToMenu devrait gérer les cas d'erreur et assurer une transition fluide.

     public void ReturnToMenu()
     {
+        try
+        {
             GameObject socket = GameObject.FindWithTag("Temporary");
-            if (socket != null) Destroy(socket.gameObject);
+            if (socket != null)
+            {
+                Destroy(socket.gameObject);
+            }
+            
+            // Transition fluide
+            StartCoroutine(LoadMenuWithFade());
-            SceneManager.LoadScene("Menu");
+        }
+        catch (System.Exception e)
+        {
+            Debug.LogError($"Erreur lors du retour au menu: {e.Message}");
+            // Chargement de secours
+            SceneManager.LoadScene("Menu");
+        }
     }

+    private IEnumerator LoadMenuWithFade()
+    {
+        // Fade out
+        yield return StartCoroutine(FadeScreen(1f));
+        // Chargement de la scène
+        SceneManager.LoadScene("Menu");
+    }

Committable suggestion was skipped due to low confidence.

Comment thread Assets/Scripts/UI/PlayerHealth.cs Outdated
Comment on lines +23 to +27
clientSocket = SocketManager.Instance.ClientSocket;
currentHealth = maxHealth;
UpdateHealthBar();

if (deadCanvas != null) deadCanvas.SetActive(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Améliorer la robustesse de l'initialisation

L'initialisation manque de vérifications importantes.

     void Start()
     {
-        clientSocket = SocketManager.Instance.ClientSocket;
+        if (SocketManager.Instance == null)
+        {
+            Debug.LogError("SocketManager.Instance est null. Assurez-vous qu'il est présent dans la scène.");
+            return;
+        }
+        clientSocket = SocketManager.Instance.ClientSocket;
         currentHealth = maxHealth;
         UpdateHealthBar();

-        if (deadCanvas != null) deadCanvas.SetActive(false);
+        if (deadCanvas == null)
+        {
+            Debug.LogWarning("deadCanvas n'est pas assigné dans l'inspecteur!");
+        }
+        else
+        {
+            deadCanvas.SetActive(false);
+        }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
clientSocket = SocketManager.Instance.ClientSocket;
currentHealth = maxHealth;
UpdateHealthBar();
if (deadCanvas != null) deadCanvas.SetActive(false);
if (SocketManager.Instance == null)
{
Debug.LogError("SocketManager.Instance est null. Assurez-vous qu'il est présent dans la scène.");
return;
}
clientSocket = SocketManager.Instance.ClientSocket;
currentHealth = maxHealth;
UpdateHealthBar();
if (deadCanvas == null)
{
Debug.LogWarning("deadCanvas n'est pas assigné dans l'inspecteur!");
}
else
{
deadCanvas.SetActive(false);
}

Comment on lines +55 to +58
private async void SocketEmitter()
{
await clientSocket.EmitAsync("rooms:end");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Gérer les erreurs de communication réseau

La méthode SocketEmitter devrait gérer les erreurs potentielles lors de l'émission.

-    private async void SocketEmitter()
+    private async void SocketEmitter()
     {
-        await clientSocket.EmitAsync("rooms:end");
+        try
+        {
+            if (clientSocket != null)
+            {
+                await clientSocket.EmitAsync("rooms:end");
+            }
+            else
+            {
+                Debug.LogError("clientSocket est null lors de l'émission de l'événement de mort");
+            }
+        }
+        catch (System.Exception e)
+        {
+            Debug.LogError($"Erreur lors de l'émission de l'événement de mort: {e.Message}");
+        }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private async void SocketEmitter()
{
await clientSocket.EmitAsync("rooms:end");
}
private async void SocketEmitter()
{
try
{
if (clientSocket != null)
{
await clientSocket.EmitAsync("rooms:end");
}
else
{
Debug.LogError("clientSocket est null lors de l'émission de l'événement de mort");
}
}
catch (System.Exception e)
{
Debug.LogError($"Erreur lors de l'émission de l'événement de mort: {e.Message}");
}
}

@AyakorK AyakorK merged commit 63971ff into develop Oct 25, 2024
@AyakorK AyakorK deleted the feature/death_scene branch October 25, 2024 12:24
@coderabbitai coderabbitai Bot mentioned this pull request Oct 25, 2024
Thomlam added a commit that referenced this pull request Oct 26, 2024
* feat: add death system

* chore: add socket emiter for end game

* fix: fix heart generator if develop don't have socket

* fix: remove crossbow
@coderabbitai coderabbitai Bot mentioned this pull request Oct 26, 2024
AyakorK added a commit that referenced this pull request Oct 27, 2024
* Chore/shadering map (#24)

* technical : add shadering and post-traitement system, adapt asset and sprite for using this

* feat: update Room2

* feat: Link the chat to the back sockets (#25)

* feat: Link chat to backend (#26)

* feat: Link the chat to the back sockets

* fix: Move the dialogue script outside the canvas to avoid it to be disabled while the canvas is going to be disabled

* fix: remove dual camera

* fix: fix crossbow system (#27)

* fix: add request to back (room name)

* fix: Revert trap manager

* hotfix: Fix things that weren't working

* feat: RPS (#29)

* feat : add multiple option and itemscollider for RockPaperCissor

* feat: Link Rock Paper Scissors to backend socket

* feat: Link RockPaperScissors, but display missing

---------

Co-authored-by: Thomas Lamiable <lamiablethomas@gmail.com>

* hotfix: Change RPS size to match the screen

* fix: fix Crossbow up sprite and arrow system, change color pixel dead font

* chore: change EmptyRoom2

* feat: add death system (#30)

* feat: add death system

* chore: add socket emiter for end game

* fix: fix heart generator if develop don't have socket

* fix: remove crossbow

* feature: add audio system and fix crossbow with sound (#32)

Co-authored-by: Guillaume MORET <90462045+AyakorK@users.noreply.github.com>

* chore: Fix env for windows (#31)

* feat: add death system

* chore: add socket emiter for end game

* fix: fix heart generator if develop don't have socket

* fix: remove crossbow

* chore: try to fix .env for Windows and Unity

* fix: Quick Fix in production

* fix: QuickFx PlayerHealth fail merge

* Fix: fix Room

* feat: Adapt Waiting Room (#33)

* feat: Instantiate waiting room when arriving in the corridor 0

* fix: Add a flag to change message one time and avoid repetition of it

* feat: Add the creation of the room when instanciating the WaitingRoom process

* feat: Try to remove the Door collider before game has started

* fix: fix lock and door system, add waiting room, last fix for dialogue system for pity

---------

Co-authored-by: Thomlam <lamiablethomas@gmail.com>

* fix: rename bear_trap to prefab

* fix: quick fix prefab room for crossbow

* fix: quickfix for bear_trap

* feat: Add room deletion when disconnection from a game

* Wip/streaming back (#35)

* feat: add streaming system with recording and sending frame jpeg

* feat: add config

* feat: first try to add socketIOSystem

* feat: Add the right socket

* feature : deploy for 60fps

---------

Co-authored-by: AyakorK <guillaume.moret@yahoo.com>

* hotfix: Get rid of a Debug

* fix: fix arrows component and stade 24 fps to streaming

* fix: Global rendering fixes (#36)

* fix: Update corridor displays + additions of colliders to avoid glitching

* fix: Add a door transition lock

* feat: Add collision to the walls in the room

* fix: Fix collisions of the hole

* fix: Fix glitched hitboxes

* fix: Destory arrows when going to an other room

---------

Co-authored-by: Guillaume MORET <90462045+AyakorK@users.noreply.github.com>
Co-authored-by: AyakorK <guillaume.moret@yahoo.com>
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.

2 participants