Skip to content

feat: Adapt Waiting Room#33

Merged
Thomlam merged 6 commits intodevelopfrom
feat/adapt_waiting_room
Oct 26, 2024
Merged

feat: Adapt Waiting Room#33
Thomlam merged 6 commits intodevelopfrom
feat/adapt_waiting_room

Conversation

@AyakorK
Copy link
Copy Markdown
Contributor

@AyakorK AyakorK commented Oct 26, 2024

Summary by CodeRabbit

  • Nouvelles Fonctionnalités

    • Ajout de plusieurs nouveaux éléments dans le prefab Main Camera, y compris HealthBar, DialogueSystem, et DeathPanel.
    • Introduction d'un nouveau prefab waitingRoomPrefab dans la scène EmptyRoom_2.
    • Ajout de nouvelles fonctionnalités de gestion d'état dans la classe WaitingTrapper.
  • Corrections de Bugs

    • Amélioration de la logique d'émission d'événements dans DoorSystem pour éviter les émissions multiples.
  • Mises à Jour de l'Interface Utilisateur

    • Modifications des propriétés de mise en page dans le prefab Dialogue_vertical pour une meilleure présentation.
    • Ajustements des paramètres de rendu et d'occlusion dans la scène Menu.
  • Améliorations de Performance

    • Réorganisation et optimisation des composants dans les scènes et prefabs pour une meilleure performance.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 26, 2024

Walkthrough

Les modifications apportées dans cette demande de tirage concernent principalement la restructuration et l'amélioration des préfabriqués dans Unity, notamment le Main Camera, CorridorX7, Door, Dialogue_vertical, et les scènes Menu et EmptyRoom_2. De nouveaux GameObjects et composants ont été ajoutés, tandis que d'autres ont été modifiés ou supprimés. Des ajustements ont également été effectués dans les scripts pour intégrer de nouvelles fonctionnalités, comme la gestion des états de jeu et les interactions avec les éléments de l'interface utilisateur.

Changes

Fichier Résumé des modifications
Assets/Prefabs/Main Camera.prefab Ajout de nouveaux GameObjects (HeartGenerator, Border, etc.), modifications du Main Camera (suppression d'un composant MonoBehaviour, mise à jour des propriétés de la caméra et du transform).
Assets/Prefabs/RoomSystem/Corridor/CorridorX7.prefab Ajout et modification de GameObjects (BlacksmithSpawnPoint, SpawnPoint, etc.), suppression d'un composant MonoBehaviour, modifications des composants Tilemap et TilemapRenderer.
Assets/Prefabs/RoomSystem/Doors/Door.prefab Mise à jour des m_TagString pour point_of_spawn et Door, modification de m_SortingLayer dans SpriteRenderer.
Assets/Prefabs/UI/DialogueBox/Dialogue_vertical.prefab Ajustements des propriétés UI (m_margin, m_AnchoredPosition, etc.), mise à jour des références d'objets.
Assets/Scenes/EmptyRoom_2.unity Ajout de waitingRoomPrefab, modifications des propriétés de divers GameObjects, mise à jour des méthodes m_OnClick.
Assets/Scenes/UI/Menu.unity Modifications des paramètres d'occlusion, des réglages de rendu, ajout de nouveaux composants (MonoBehaviour, VideoPlayer).
Assets/Scripts/GameTracker.cs Ajout de waitingRoomPrefab, gestion de l'instanciation de la salle d'attente dans NextStage.
Assets/Scripts/Map Control/DoorSystem.cs Ajout de isGameCreated pour contrôler l'émission d'événements de socket lors de l'instanciation de la salle.
Assets/Scripts/UI/Dialogue/DialogueManager.cs Ajout de déclarations de journalisation pour le débogage dans Update et ProcessMessageQueue.
Assets/Scripts/UI/MainMenu.cs Suppression de l'émission asynchrone de l'événement "rooms:create" dans StartRoom.
Assets/Scripts/WaitingTrapper.cs Ajout de variables pour la gestion de l'état du jeu, mise à jour de la méthode OnStartGame pour gérer les événements de socket.
ProjectSettings/TagManager.asset Ajout d'un nouveau tag SpawnPoint.

Possibly related PRs

  • feat: Add Socket to props to send them to back-end #8: Modifications significatives des préfabriqués, y compris l'ajout de nouveaux composants, indiquant une relation potentielle avec la gestion des préfabriqués.
  • Feature/add traps #19: Introduction d'une classe Bear_Trap pour gérer les interactions avec un piège, pertinent par rapport aux éléments de gestion de la santé dans le PR principal.
  • Chore/fix env for windows #31: Ajout d'une référence trapPrefabs dans la scène EmptyRoom_2, en lien avec l'intégration des nouveaux GameObjects.

Suggested reviewers

  • Thomlam

🐰 Dans le jardin des préfabriqués,
De nouveaux amis ont pris leur envol,
Coeurs et dialogues, tous réunis,
Dans le monde du jeu, un grand contrôle.
Les portes s'ouvrent, les salles s'éclairent,
Avec des tags et des scripts à faire,
Oh, quelle fête pour le développeur,
Avec des changements qui mettent en valeur ! 🌟


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.

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: 7

🧹 Outside diff range and nitpick comments (10)
ProjectSettings/TagManager.asset (1)

14-14: Suggestion : Documenter l'utilisation du tag

Il serait utile de documenter l'utilisation prévue de ce tag dans le README du projet ou dans la documentation technique, en précisant son rôle dans les prefabs CorridorX7 et Door.

Assets/Scripts/Map Control/DoorSystem.cs (2)

15-16: Suggestion de renommage de la variable pour plus de clarté

La variable isGameCreated pourrait avoir un nom plus descriptif comme hasEmittedRoomCreation ou isRoomCreationEventSent pour mieux refléter son utilisation spécifique dans le contrôle d'émission d'événements socket.


Line range hint 1-100: Recommandations architecturales

La classe RoomManager gère actuellement plusieurs responsabilités :

  • Gestion des salles
  • Communication socket
  • Gestion des pièges
  • Positionnement du joueur

Suggestions d'amélioration architecturale :

  1. Séparer la gestion des sockets dans un service dédié
  2. Créer un gestionnaire d'état de jeu distinct
  3. Extraire la logique de gestion des pièges dans une classe dédiée

Voulez-vous que je propose une structure de code détaillée pour cette refactorisation ?

Assets/Scripts/GameTracker.cs (1)

22-23: Ajoutez des commentaires XML pour documenter les nouvelles variables.

Les nouvelles variables waitingRoomPrefab et waitingRoom manquent de documentation expliquant leur utilité et leur relation.

Suggestion d'amélioration :

+    /// <summary>
+    /// Préfab de la salle d'attente à instancier au début du jeu
+    /// </summary>
     public GameObject waitingRoomPrefab;

+    /// <summary>
+    /// Instance active de la salle d'attente
+    /// </summary>
     private GameObject waitingRoom;

Also applies to: 32-33

Assets/Scripts/UI/Dialogue/DialogueManager.cs (1)

Line range hint 1-150: Suggestion pour améliorer la gestion des logs

Il serait judicieux d'implémenter un système de logging plus robuste avec différents niveaux de verbosité (DEBUG, INFO, WARNING, ERROR) et la possibilité de désactiver les logs en production.

Je suggère de créer une classe utilitaire dédiée au logging. Voici un exemple d'implémentation :

public static class Logger 
{
    private static bool isDebugEnabled = Debug.isDebugBuild;

    public static void LogDebug(string message, Object context = null) 
    {
        if (isDebugEnabled)
        {
            Debug.Log($"[DEBUG] {message}", context);
        }
    }

    public static void LogInfo(string message, Object context = null) 
    {
        Debug.Log($"[INFO] {message}", context);
    }
}
Assets/Scripts/WaitingTrapper.cs (2)

46-49: Ajouter des logs pour faciliter le débogage

Il serait utile d'ajouter des logs pour tracer l'état du jeu au démarrage.

 private void OnStartGame()
 {
+    Debug.Log("Game starting - unlocking door and updating state");
     gameStart = true;
     unlockDoor();
 }

Line range hint 1-204: Considérer une refactorisation architecturale

La classe WaitingTrapper gère actuellement plusieurs responsabilités :

  • Gestion de l'interface utilisateur
  • Gestion des états du jeu
  • Communication réseau
  • Gestion des portes

Considérez de séparer ces responsabilités en plusieurs classes distinctes suivant le principe de responsabilité unique (SRP) :

  • WaitingRoomUI pour la gestion de l'interface
  • WaitingRoomState pour la gestion des états
  • WaitingRoomNetworking pour la communication
  • DoorController pour la gestion des portes
Assets/Prefabs/UI/DialogueBox/Dialogue_vertical.prefab (2)

Line range hint 104-112: Attention à la lisibilité du texte

La taille de police de 0.1 semble très petite et l'auto-sizing est désactivé. Cela pourrait poser des problèmes de lisibilité.

Suggestions d'amélioration :

- propertyPath: m_fontSize
- value: 0.1
+ propertyPath: m_fontSize
+ value: 0.3
- propertyPath: m_enableAutoSizing
- value: 0
+ propertyPath: m_enableAutoSizing
+ value: 1

Line range hint 113-140: Considérer l'internationalisation du titre

Le système de dialogue est bien configuré avec les sprites, mais le titre "Mort" devrait peut-être être géré via un système de localisation pour supporter plusieurs langues.

Je suggère d'utiliser un système de localisation comme Unity Localization pour gérer les textes du dialogue.

Assets/Prefabs/RoomSystem/Corridor/CorridorX7.prefab (1)

Line range hint 1-24: Améliorer la structure des points de spawn

La configuration actuelle des points de spawn pourrait être améliorée :

  1. Considérez la création d'un système d'énumération pour les types de spawn
  2. Standardisez les tags entre "Respawn" et "SpawnPoint"
  3. Regroupez les points de spawn dans un GameObject parent dédié

Proposition de refactorisation :

-  m_TagString: Respawn
+  m_TagString: SpawnPoint

Considérez également la création d'un composant personnalisé :

public enum SpawnPointType {
    Player,
    Blacksmith,
    Altar
}

public class SpawnPoint : MonoBehaviour {
    public SpawnPointType type;
}

Also applies to: 417-450

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 9f7f520 and dbb09c8.

📒 Files selected for processing (12)
  • Assets/Prefabs/Main Camera.prefab (0 hunks)
  • Assets/Prefabs/RoomSystem/Corridor/CorridorX7.prefab (3 hunks)
  • Assets/Prefabs/RoomSystem/Doors/Door.prefab (2 hunks)
  • Assets/Prefabs/UI/DialogueBox/Dialogue_vertical.prefab (1 hunks)
  • Assets/Scenes/EmptyRoom_2.unity (2 hunks)
  • Assets/Scenes/UI/Menu.unity (2 hunks)
  • Assets/Scripts/GameTracker.cs (3 hunks)
  • Assets/Scripts/Map Control/DoorSystem.cs (2 hunks)
  • Assets/Scripts/UI/Dialogue/DialogueManager.cs (2 hunks)
  • Assets/Scripts/UI/MainMenu.cs (0 hunks)
  • Assets/Scripts/WaitingTrapper.cs (2 hunks)
  • ProjectSettings/TagManager.asset (1 hunks)
💤 Files with no reviewable changes (2)
  • Assets/Prefabs/Main Camera.prefab
  • Assets/Scripts/UI/MainMenu.cs
🔇 Additional comments (12)
ProjectSettings/TagManager.asset (1)

14-14: Ajout du tag SpawnPoint validé !

L'ajout de ce nouveau tag suit la convention de nommage Unity et s'intègre bien avec les tags existants.

Assets/Scripts/GameTracker.cs (1)

51-54: ⚠️ Potential issue

Vérifiez la logique de réinitialisation de la salle d'attente.

La salle d'attente est instanciée uniquement quand stageCounter == 0, mais il n'y a pas de logique pour la détruire. Cela pourrait créer des instances multiples si le jeu est réinitialisé.

Suggestion d'amélioration :

     if (stageCounter == 0)
     {
+        if (waitingRoom != null)
+        {
+            Destroy(waitingRoom);
+        }
         instantiateWaitingRoom();
     }
Assets/Prefabs/RoomSystem/Doors/Door.prefab (1)

16-16: Modification appropriée du tag pour le point de spawn.

Le changement de tag de "Untagged" à "SpawnPoint" permet une meilleure identification des points de spawn dans le système de salles.

Assets/Scripts/WaitingTrapper.cs (1)

42-44: Bonne implémentation des événements socket

L'ajout de l'événement "rooms:start" est bien structuré et suit les bonnes pratiques d'organisation des gestionnaires d'événements.

Assets/Prefabs/UI/DialogueBox/Dialogue_vertical.prefab (3)

41-41: Configuration de base correcte !

Le nom et le tag du prefab sont cohérents avec sa fonction de dialogue vertical.


Line range hint 141-144: Vérifier l'impact de la suppression du composant

Un GameObject a été supprimé du prefab. Il faut s'assurer que cette suppression n'affecte pas le fonctionnement du dialogue.

#!/bin/bash
# Rechercher les références potentielles au composant supprimé
rg "4919147100269876766" --type cs

Line range hint 88-95: Vérifier la réactivité de l'interface

Les modifications de positionnement et de taille sont précises, mais il serait prudent de vérifier le comportement sur différentes résolutions d'écran.

Also applies to: 96-103

✅ Verification successful

The previous command might have failed due to permission issues. Let's try a different approach to gather information about UI layouts and responsiveness in the codebase.


L'interface est correctement configurée pour la réactivité

D'après l'analyse des configurations d'ancrage dans les prefabs UI, je constate que :

  • Les éléments utilisent un mélange approprié d'ancrages fixes (0.5, 0.5) pour le positionnement précis et d'ancrages étirés (0,0 à 1,1) pour la mise à l'échelle
  • Les valeurs de m_SizeDelta sont définies de manière cohérente pour maintenir les proportions
  • Les éléments enfants héritent correctement des paramètres de mise à l'échelle de leurs parents

La configuration actuelle permet une adaptation appropriée aux différentes résolutions d'écran.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Rechercher d'autres prefabs UI pour comparer les configurations d'ancrage
fd -e prefab -x grep -l "m_AnchorMin\|m_AnchorMax" {} \; 2>/dev/null

Length of output: 34


Script:

#!/bin/bash
# Search for UI prefabs and their anchor configurations
fd -e prefab -exec cat {} \; | grep -A 2 -B 2 "m_AnchorMin\|m_AnchorMax"

# Search for any UI scaling or responsive layout scripts
rg -t cs "Canvas|RectTransform|CanvasScaler" -A 5 -B 5

Length of output: 49220


Script:

#!/bin/bash
# Rechercher les configurations de CanvasScaler qui gèrent la mise à l'échelle de l'interface
rg -t cs "CanvasScaler" -A 5 -B 5

# Rechercher les configurations d'ancrage dans les prefabs UI
fd -e prefab -x grep -A 3 -B 3 "m_AnchorMin\|m_AnchorMax\|m_SizeDelta" {} \;

Length of output: 22199

Assets/Scenes/EmptyRoom_2.unity (2)

173-173: Ajout cohérent du waitingRoomPrefab

L'ajout de cette référence s'intègre bien avec les autres prefabs de progression du jeu.


182-185: Vérifier l'impact de la désactivation du mini-jeu

La désactivation initiale du RockPaperCissorGame pourrait affecter le flux du jeu. Assurez-vous que cette modification est intentionnelle et qu'elle est gérée correctement dans le code.

✅ Verification successful

La désactivation initiale est cohérente avec la logique du jeu

L'analyse du code montre que le mini-jeu est conçu pour être désactivé après utilisation via la méthode CloseCanvas(). La désactivation initiale (m_IsActive = 0) est donc cohérente avec ce comportement, car le jeu sera activé uniquement lorsque nécessaire.

Points clés confirmés :

  • Le canvas est explicitement désactivé après chaque utilisation
  • La gestion de l'état actif/inactif est correctement implémentée
  • Le flux de jeu n'est pas impacté négativement
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Rechercher les références au RockPaperCissorGame pour vérifier sa gestion
rg -l "RockPaperCissorGame" --type cs

Length of output: 37


Script:

#!/bin/bash
# Search for any references to rock, paper, scissors game with different possible naming patterns
rg -l -i "rock.*paper|pierre.*feuille|chifoumi|shifumi" --type cs

# Search for any game state or activation related patterns
rg -i "rock.*paper.*active|pierre.*feuille.*active" --type cs -A 5

# Look for prefab or scene references
fd -e prefab -e unity | xargs rg "guid: a29b258ccf5dc90498df6e925ee2a3e5" -l

Length of output: 12362


Script:

#!/bin/bash
# Let's examine the RockPaperScissors.cs file content
cat Assets/Scripts/RockPaperScissors.cs

# Check for any activation/deactivation logic in the script
rg "SetActive|gameObject.active" Assets/Scripts/RockPaperScissors.cs -A 3

Length of output: 2751

Assets/Prefabs/RoomSystem/Corridor/CorridorX7.prefab (2)

2248-2251: Vérifier la référence au nouveau prefab de salle

La modification de la référence newRoomPrefab et du tag SpawnPoint fait partie d'une mise à jour plus large. Assurez-vous que :

  1. Le prefab référencé existe bien
  2. Le tag est correctement défini dans le TagManager
#!/bin/bash
# Description: Vérifier l'existence du prefab et du tag

# Vérifier l'existence du prefab
fd "35bed5d2a54129845bcff6b3abaa746b" "Assets"

# Vérifier la définition du tag
rg "SpawnPoint" "ProjectSettings/TagManager.asset"

1289-1289: Vérifier la cohérence des sorting layers

La modification de la couche de tri des murs (passage à la couche 4) pourrait affecter l'ordre de rendu. Assurez-vous que cela reste cohérent avec les autres prefabs du système de salles.

✅ Verification successful

La modification de la couche de tri est cohérente

L'utilisation de la couche de tri 4 (Ground+1) est cohérente avec les autres prefabs du système de salles. On observe le même motif dans Room.prefab et CorridorX7.prefab, où la couche 4 est utilisée de manière similaire. Cette configuration correspond bien à la définition des couches de tri dans le TagManager.asset, où la couche 4 (Ground+1) est destinée aux éléments qui doivent être rendus au-dessus du niveau de base.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Vérifier la cohérence des sorting layers dans les autres prefabs
# Rechercher tous les TilemapRenderer avec le même SortingLayerID

rg -l "m_SortingLayerID: -1791178601" "Assets/Prefabs/RoomSystem"

Length of output: 108


Script:

#!/bin/bash
# Search for sorting layer values in corridor prefabs
rg "m_SortingLayer: \d+" "Assets/Prefabs/RoomSystem/Corridor/"

# Also check the TagManager for sorting layer definitions
cat "ProjectSettings/TagManager.asset"

Length of output: 1499

Assets/Scenes/UI/Menu.unity (1)

1137-1137: Vérifiez que le nouveau MonoBehaviour est correctement configuré sur la Main Camera

La Main Camera a un nouveau composant MonoBehaviour ajouté (lignes 1137 et 1219-1262). Assurez-vous que le script référencé par m_Script avec le GUID a79441f348de89743a2939f4d699eac1 existe dans le projet et est correctement assigné.

Pour confirmer cela, vous pouvez exécuter le script suivant pour rechercher le script correspondant :

Also applies to: 1219-1262

Comment on lines +61 to +66
if (!isGameCreated)
{
isGameCreated = true;
clientSocket.Emit("rooms:create");
}

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

Suggestions d'amélioration pour la gestion des événements socket

  1. La structure du code pourrait être améliorée pour réduire l'imbrication des conditions.
  2. Il manque une gestion des erreurs pour l'émission des événements socket.

Voici une suggestion de refactorisation :

 if (newRoom.name.Contains("CorridorX7"))
 {
-    if (!isGameCreated)
-    {
-        isGameCreated = true;
-        clientSocket.Emit("rooms:create");
-    }
+    EmitRoomCreationEventOnce();
     gameTracker.SpawnCorridorEvents();
     gameTracker.NextStage();
     isInCorridorX7 = true;
     SocketEmitter();
 }

Ajoutez cette nouvelle méthode :

private async void EmitRoomCreationEventOnce()
{
    if (isGameCreated) return;
    
    try 
    {
        await clientSocket.EmitAsync("rooms:create");
        isGameCreated = true;
    }
    catch (System.Exception e)
    {
        Debug.LogError($"Erreur lors de l'émission de l'événement rooms:create : {e.Message}");
    }
}

Comment on lines +59 to +67
private void instantiateWaitingRoom()
{
waitingRoom = GameObject.Find("WaitingRoom");
if (waitingRoom == null && waitingRoomPrefab != null)
{
Instantiate(waitingRoomPrefab, new Vector3(0, 0, 0), Quaternion.identity);
}
}

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

Optimisez la recherche de la salle d'attente.

La méthode instantiateWaitingRoom() présente plusieurs problèmes :

  1. GameObject.Find() est coûteux en performances
  2. Absence de vérification de la position d'instanciation
  3. Pas de gestion des erreurs si waitingRoomPrefab est null

Suggestion de refactorisation :

     private void instantiateWaitingRoom()
     {
-        waitingRoom = GameObject.Find("WaitingRoom");
-        if (waitingRoom == null && waitingRoomPrefab != null)
+        if (waitingRoomPrefab == null)
+        {
+            Debug.LogError("Le préfab de la salle d'attente n'est pas assigné !");
+            return;
+        }
+
+        if (waitingRoom != null)
         {
-            Instantiate(waitingRoomPrefab, new Vector3(0, 0, 0), Quaternion.identity);
+            Debug.LogWarning("Une instance de la salle d'attente existe déjà !");
+            return;
         }
+
+        waitingRoom = Instantiate(waitingRoomPrefab, Vector3.zero, Quaternion.identity);
+        if (waitingRoom == null)
+        {
+            Debug.LogError("Échec de l'instanciation de la salle d'attente !");
+        }
     }
📝 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 void instantiateWaitingRoom()
{
waitingRoom = GameObject.Find("WaitingRoom");
if (waitingRoom == null && waitingRoomPrefab != null)
{
Instantiate(waitingRoomPrefab, new Vector3(0, 0, 0), Quaternion.identity);
}
}
private void instantiateWaitingRoom()
{
if (waitingRoomPrefab == null)
{
Debug.LogError("Le préfab de la salle d'attente n'est pas assigné !");
return;
}
if (waitingRoom != null)
{
Debug.LogWarning("Une instance de la salle d'attente existe déjà !");
return;
}
waitingRoom = Instantiate(waitingRoomPrefab, Vector3.zero, Quaternion.identity);
if (waitingRoom == null)
{
Debug.LogError("Échec de l'instanciation de la salle d'attente !");
}
}

m_LightmapParameters: {fileID: 0}
m_SortingLayerID: -1791178601
m_SortingLayer: 3
m_SortingLayer: 4
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

Attention : Incohérence dans les couches de rendu détectée

La modification du SortingLayer de 3 à 4 pour Door.prefab n'est pas totalement cohérente avec l'ensemble des préfabriqués. Les résultats montrent une division :

  • SortingLayer 4 : Door.prefab, CorridorX7.prefab, Room.prefab
  • SortingLayer 3 : Forgeron.prefab, Altar.prefab

Cette différence pourrait créer des problèmes de rendu avec les éléments d'événements de corridor (Forgeron, Altar). Une harmonisation complète des SortingLayers devrait être envisagée.

🔗 Analysis chain

Ajustement cohérent de la couche de rendu.

La modification du SortingLayer de 3 à 4 maintient la cohérence visuelle avec les autres préfabriqués, notamment le CorridorX7.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Vérifier la cohérence des SortingLayers dans les préfabriqués connexes
# Test: Rechercher d'autres modifications de SortingLayer pour assurer la cohérence

rg -l "m_SortingLayer: [34]" "Assets/Prefabs/RoomSystem"

Length of output: 309


Script:

#!/bin/bash
# Vérifions la valeur exacte du SortingLayer dans chaque fichier
rg "m_SortingLayer: [34]" -A 1 "Assets/Prefabs/RoomSystem"

Length of output: 1226


if (godPrefabDictionary.TryGetValue(godId, out GodInfo godInfo))
{
Debug.Log("NTR");
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 qualité des messages de débogage

Les messages de débogage "NTR" ne sont pas descriptifs et n'apportent aucune information utile pour le débogage. De plus, ils risquent de polluer les logs en production.

Suggestions d'amélioration :

- Debug.Log("NTR");
+ Debug.Log($"Message traité pour le dieu {godId}: {messageText.Substring(0, Math.Min(messageText.Length, 50))}...");
- Debug.Log("NTR");
+ Debug.Log($"Affichage du dialogue: {nextMessage.title} - {nextMessage.dialogue.Substring(0, Math.Min(nextMessage.dialogue.Length, 50))}...");

Also applies to: 106-106

Comment on lines +179 to +192
spawnOfPoint = GameObject.FindGameObjectWithTag("SpawnPoint");
if (gameStart)
{
trapperJoined = false; // Reset the flag
gameStart = false;
coroutineStarted = true;
StartCoroutine(StartCountdown(5)); // Start countdown for traps
}

if (gamePaused && !coroutineStarted)
if ( spawnOfPoint == GameObject.FindGameObjectWithTag("SpawnPoint") && !coroutineStarted){
lockDoor();
}
if (gamePaused && !coroutineStarted && !messageSet)
{
messageText.text = "Waiting for trapper to join...";
messageText.text = trapperJoined ? "Waiting for the game to start..." : "Waiting for trappers to join...";
messageSet = true;
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

Optimiser les performances de la méthode Update

L'utilisation de FindGameObjectWithTag à chaque frame est coûteuse en performances. De plus, la logique de vérification peut être simplifiée.

 void Update()
 {
-    spawnOfPoint = GameObject.FindGameObjectWithTag("SpawnPoint");
-    if (gameStart)
+    if (gameStart && !coroutineStarted)
     {
         gameStart = false;
         coroutineStarted = true;
         StartCoroutine(StartCountdown(5));
     }
-    if ( spawnOfPoint == GameObject.FindGameObjectWithTag("SpawnPoint") && !coroutineStarted){
+    if (spawnOfPoint != null && !coroutineStarted) {
         lockDoor();
     }
     if (gamePaused && !coroutineStarted && !messageSet)
     {
         messageText.text = trapperJoined ? "Waiting for the game to start..." : "Waiting for trappers to join...";
         messageSet = true;
     }

Considérez également de déplacer la recherche du SpawnPoint dans Start() ou Awake() :

void Awake()
{
    spawnOfPoint = GameObject.FindGameObjectWithTag("SpawnPoint");
    if (spawnOfPoint == null)
    {
        Debug.LogError("SpawnPoint non trouvé lors de l'initialisation!");
    }
}

Comment on lines +26 to +30
private bool gameStart = false;
private bool messageSet = false;

public GameObject spawnOfPoint;

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

Attention à la visibilité publique de spawnOfPoint

Il est préférable d'utiliser [SerializeField] plutôt qu'une variable publique pour maintenir l'encapsulation tout en permettant l'assignation depuis l'éditeur Unity.

-    public GameObject spawnOfPoint;
+    [SerializeField]
+    private GameObject spawnOfPoint;
📝 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 bool gameStart = false;
private bool messageSet = false;
public GameObject spawnOfPoint;
private bool gameStart = false;
private bool messageSet = false;
[SerializeField]
private GameObject spawnOfPoint;

Comment on lines +71 to +86
private void lockDoor()
{
if (spawnOfPoint == null)
{
Debug.LogError("Spawn point not found!");
return;
}
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = false;
}


private void unlockDoor()
{
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = true;
Destroy(overlay.transform.parent.gameObject);
Destroy(this);
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 gestion des erreurs et la destruction des objets

Plusieurs points à améliorer :

  1. La méthode unlockDoor devrait vérifier si les objets existent avant de les détruire
  2. La destruction de this pourrait causer des problèmes si d'autres méthodes sont en cours d'exécution
 private void unlockDoor()
 {
+    if (spawnOfPoint == null)
+    {
+        Debug.LogError("Spawn point not found in unlockDoor!");
+        return;
+    }
     spawnOfPoint.GetComponent<BoxCollider2D>().enabled = true;
-    Destroy(overlay.transform.parent.gameObject);
-    Destroy(this);
+    if (overlay != null && overlay.transform.parent != null)
+    {
+        Destroy(overlay.transform.parent.gameObject);
+    }
+    StartCoroutine(DestroyAfterFrame());
 }

+private IEnumerator DestroyAfterFrame()
+{
+    yield return new WaitForEndOfFrame();
+    Destroy(this);
+}
📝 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 void lockDoor()
{
if (spawnOfPoint == null)
{
Debug.LogError("Spawn point not found!");
return;
}
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = false;
}
private void unlockDoor()
{
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = true;
Destroy(overlay.transform.parent.gameObject);
Destroy(this);
private void lockDoor()
{
if (spawnOfPoint == null)
{
Debug.LogError("Spawn point not found!");
return;
}
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = false;
}
private void unlockDoor()
{
if (spawnOfPoint == null)
{
Debug.LogError("Spawn point not found in unlockDoor!");
return;
}
spawnOfPoint.GetComponent<BoxCollider2D>().enabled = true;
if (overlay != null && overlay.transform.parent != null)
{
Destroy(overlay.transform.parent.gameObject);
}
StartCoroutine(DestroyAfterFrame());
}
private IEnumerator DestroyAfterFrame()
{
yield return new WaitForEndOfFrame();
Destroy(this);
}

@AyakorK AyakorK requested a review from Thomlam October 26, 2024 11:31
@Thomlam Thomlam merged commit dee7b54 into develop Oct 26, 2024
@Thomlam Thomlam deleted the feat/adapt_waiting_room branch October 26, 2024 12:11
Thomlam added a commit that referenced this pull request Oct 26, 2024
* 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>
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