Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The merge #143

Merged
merged 239 commits into from
Nov 5, 2023
Merged

The merge #143

merged 239 commits into from
Nov 5, 2023

Conversation

Saverio976
Copy link
Collaborator

@Saverio976 Saverio976 commented Nov 5, 2023

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Norm of the code has been respected
  • In what state is your pull request?
  • Ready to merge / Waiting a reviwer to see my work.
  • Work In Progress (WIP) / My work is not finish but i want daily reviews. (Draft)
  • CI Review / I want to see what the CI thinks of my work.
  • Need feedback / Just to have feedback on what i produced. (May not be merged)
  • What kind of change does this PR introduce? (You can choose multiple)
  • Bug fix
  • Feature request
  • New / Updated documentation
  • Testing CI ( Make the pull request in draft mode)
  • What is the current behavior? (link an issue based on the kind of change this pr introduce)

  • What is the new behavior (if this is a feature change)?

  • Other information:

Summary by CodeRabbit

  • Documentation Changes
    • Updated developer guide to reflect changes in function signatures and protocol specifications.
  • Build and Configuration Changes
    • Modified build process to include a non-parallel option.
    • Updated CMakeLists.txt files to include new source files and directories.
  • New Features
    • Introduced new client systems for handling game events and animations.
    • Added a function to display available IP addresses on the server.
  • Bug Fixes
    • Improved signal handling in the main server to ensure proper shutdown.

KitetsuK and others added 30 commits October 24, 2023 14:49
…B/R-Bus into feature/RB-142-menu-de-connection
…B/R-Bus into feature/RB-142-menu-de-connection
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

coderabbitai bot commented Nov 5, 2023

Walkthrough

A high-level summary of the overall changes is as follows:

The changes include documentation updates, build and configuration changes, and client systems changes. In the documentation, the signature of a function in the Text class has been modified, and the protocol specification for a game has been updated. The build process has been modified to allow for parallel or single-threaded builds based on command-line arguments. The CMakeLists.txt files have been updated to include necessary directories and source files. In the client systems, new files and functions have been added, and existing functions have been modified to handle game events, graphics, menus, and networking.

Changes

File Summary
Documentation Changes
docs/developer-guide/client/graphic/text.md Modified the signature of the setCurrentText function in the Text class to setText.
docs/network/rfc/RFC.md Updated the protocol specification for a game, introducing new actions, modifying existing actions, and updating action headers.
Build and Configuration Changes
scripts/compil.sh Modified the build process to run in parallel or single-threaded based on command-line arguments.
src/CMakeLists.txt Added a new target_include_directories command to include the current source directory for the server target.
src/Game/CMakeLists.txt Added two new source files, "WaveSystemsAll.cpp" and "PhysicSystems.cpp", to the target sources.
src/Server/CMakeLists.txt Adjusted the indentation of the target_include_directories command.
src/Server/Systems/CMakeLists.txt Added a new source file called "WaveSystem.cpp" to the target.
Client Systems Changes
src/Client/Systems/ClientSystems.cpp Added a new file "ClientSystems.hpp" with a namespace "Systems" containing a function "getSystemsGroups()".
src/Client/Systems/ClientSystems.hpp Added a new header file with a namespace "Systems" containing a function "getSystemsGroups".
src/Client/Systems/CustomTypes/AnimRect.hpp Added a new header file defining a class "AnimRect" for handling animated rectangles.
src/Client/Systems/Events/EventsSystems.cpp Added functionality for handling end game events and modifying the end game text.
src/Client/Systems/Events/EventsSystems.hpp Added a new function handleEndGameEvent to the EventsSystems namespace.
src/Client/Systems/Graphic/GraphicSystems.cpp Added a new function debugCollisionRect to the Systems namespace.
src/Client/Systems/Menus/CreateLobby/CMakeLists.txt Added a CMakeLists.txt file for the Create Lobby menu.
src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp Added various functions for button callbacks and menu navigation.
src/Client/Systems/Menus/Menu/Menu.cpp Modified functions for initializing menu entities and handling input.
src/Client/Systems/Menus/Menu/MenuSystems.cpp Added and modified functions for handling input, UI elements, and scene management.
src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.cpp Added and modified functions for initializing the Select Lobby menu and handling lobby pages.
src/Client/Systems/Network/ClientNetwork.cpp Added and modified functions for handling network messages and connections.
src/Game/CreateMissiles.cpp Added and modified functions for creating and updating missiles.
src/Game/GameCustomTypes.cpp Added and modified custom types and functions for game systems.
src/Game/GameSystems.cpp Modified functions for initializing game entities and handling game events.
src/Game/PhysicSystems.cpp Added and modified functions for handling physics in the game.
src/Game/ResourcesManager.hpp Modified the JsonType enum and the ResourcesManagerValues::paths vector.
src/Game/WaveSystemsAll.cpp Added and modified functions for handling waves in the game.
src/Nitwork/ANitwork.cpp Modified the StartInputHandler function and added a new deletePacketFromEndPoints function.
src/Nitwork/ANitwork.hpp Added a new function deletePacketFromEndPoints to the ANitwork class.
src/Nitwork/Nitwork.h Modified the maximum number of clients for the main server and added new action types.
src/Nitwork/NitworkClient.cpp Added and modified functions for connecting to the server and handling network messages.
src/Nitwork/NitworkMainServer.cpp Added and modified functions for handling lobby creation and management.
src/Nitwork/NitworkMainServer.hpp Added new functions and member variables for handling lobby creation and management.
src/Nitwork/NitworkServer.cpp Added and modified functions for handling network messages and lobby management.
src/Nitwork/NitworkServer.hpp Added new functions and member variables for handling network messages and lobby management.
src/Server/Systems/Network/ServerNetwork.cpp Modified functions for handling network messages and player missiles.
src/Server/Systems/WaveSystem.cpp Added and modified functions for handling waves in the server.
src/Server/Systems/WaveSystem.hpp Added a new header file defining a class and a function for wave handling.
src/init.cpp Added and modified functions for scene management and network messages.
src/init.hpp Added new values to enums and modified function calls.

"As the leaves fall, so do the bugs, 🍂🐞
In the code, we've made some tugs. 💻🔧
New actions added, old ones refined, 🎮✨
For a gaming experience, one of a kind. 🕹️🏆
So here's to the coders, working day and night, 🌞🌜
Making sure every pixel is just right. 🖥️👌
As we hop into the season, let's give a cheer, 🐇🎉
For the changes made, far and near!" 🌍🍻


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • 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/coderabbit-overrides.v2.json

Copy link
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.

Review Status

Actionable comments generated: 46

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 198c305 and 75b39ff.
Files ignored due to filter (46)
  • assets/Audio/Musics/Napstablook.mp3
  • assets/Audio/Musics/Papyrus.mp3
  • assets/Boss/big-purple-boss.png
  • assets/Boss/green-head-boss.png
  • assets/Boss/le-jutteur-boss.png
  • assets/Boss/napstablook.png
  • assets/Boss/papyrus.png
  • assets/Json/HUD.json
  • assets/Json/bullets.json
  • assets/Json/createLobby.json
  • assets/Json/enemies.json
  • assets/Json/menu.json
  • assets/Json/parallaxData2.json
  • assets/Json/playerData.json
  • assets/Json/selectLobby.json
  • assets/Json/terminatorData.json
  • assets/Json/wave.json
  • assets/Json/waves.json
  • assets/Map/parallax2/background-stars.png
  • assets/Map/parallax2/black-background.png
  • assets/Map/parallax2/parallax2-firstImage.png
  • assets/Map/parallax2/parallax2-secondImage.png
  • assets/Map/parallax2/planet_blue.png
  • assets/Map/parallax2/planet_fire.png
  • assets/Map/parallax2/planet_green.png
  • assets/Map/parallax2/planet_grey.png
  • assets/Map/parallax2/porthole.png
  • assets/Map/parallax2/porthole2.png
  • assets/Map/parallax2/stars.png
  • assets/Menu/bix-box.png
  • assets/Menu/button.png
  • assets/Menu/input-box.png
  • assets/Menu/text-box.png
  • assets/R-TypeSheet/bee_1.png
  • assets/R-TypeSheet/big-daemon.png
  • assets/R-TypeSheet/blue boos.png
  • assets/R-TypeSheet/classic-bullets.png
  • assets/R-TypeSheet/crawler-attack.png
  • assets/R-TypeSheet/daemon.png
  • assets/R-TypeSheet/dragon.png
  • assets/R-TypeSheet/hornet-attack.png
  • assets/R-TypeSheet/jutteurs-bullet.png
  • assets/R-TypeSheet/powerUp.png
  • assets/R-TypeSheet/rat-attack.png
  • assets/R-TypeSheet/scorpion-attack.png
  • assets/R-TypeSheet/spaceship.png
Files selected for processing (57)
  • docs/developer-guide/client/graphic/text.md (1 hunks)
  • docs/network/rfc/RFC.md (25 hunks)
  • scripts/compil.sh (1 hunks)
  • src/CMakeLists.txt (1 hunks)
  • src/Client/Systems/CMakeLists.txt (1 hunks)
  • src/Client/Systems/ClientSystems.cpp (1 hunks)
  • src/Client/Systems/ClientSystems.hpp (1 hunks)
  • src/Client/Systems/CustomTypes/AnimRect.hpp (1 hunks)
  • src/Client/Systems/Events/EventsSystems.cpp (6 hunks)
  • src/Client/Systems/Events/EventsSystems.hpp (1 hunks)
  • src/Client/Systems/Graphic/GraphicSystems.cpp (1 hunks)
  • src/Client/Systems/Menus/CMakeLists.txt (1 hunks)
  • src/Client/Systems/Menus/CreateLobby/CMakeLists.txt (1 hunks)
  • src/Client/Systems/Menus/CreateLobby/CreateLobbySystems.cpp (1 hunks)
  • src/Client/Systems/Menus/CreateLobby/CreateLobbySystems.hpp (1 hunks)
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp (1 hunks)
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.hpp (1 hunks)
  • src/Client/Systems/Menus/Menu/Menu.cpp (3 hunks)
  • src/Client/Systems/Menus/Menu/Menu.hpp (3 hunks)
  • src/Client/Systems/Menus/Menu/MenuSystems.cpp (9 hunks)
  • src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.cpp (2 hunks)
  • src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.hpp (1 hunks)
  • src/Client/Systems/Network/ClientNetwork.cpp (11 hunks)
  • src/Client/Systems/Network/ClientNetwork.hpp (1 hunks)
  • src/Client/Systems/Parallax/Parallax.cpp (1 hunks)
  • src/Game/CMakeLists.txt (2 hunks)
  • src/Game/CreateMissiles.cpp (4 hunks)
  • src/Game/CreateMissiles.hpp (1 hunks)
  • src/Game/GameCustomTypes.cpp (1 hunks)
  • src/Game/GameCustomTypes.hpp (4 hunks)
  • src/Game/GameSystems.cpp (8 hunks)
  • src/Game/PhysicSystems.cpp (1 hunks)
  • src/Game/PhysicSystems.hpp (1 hunks)
  • src/Game/ResourcesManager.hpp (1 hunks)
  • src/Game/WaveSystemsAll.cpp (1 hunks)
  • src/Game/WaveSystemsAll.hpp (1 hunks)
  • src/Nitwork/ANitwork.cpp (2 hunks)
  • src/Nitwork/ANitwork.hpp (3 hunks)
  • src/Nitwork/MessageTypes.h (1 hunks)
  • src/Nitwork/Nitwork.h (8 hunks)
  • src/Nitwork/NitworkClient.cpp (14 hunks)
  • src/Nitwork/NitworkClient.hpp (18 hunks)
  • src/Nitwork/NitworkMainServer.cpp (7 hunks)
  • src/Nitwork/NitworkMainServer.hpp (6 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
  • src/Nitwork/NitworkServer.hpp (5 hunks)
  • src/Server/CMakeLists.txt (1 hunks)
  • src/Server/Systems/CMakeLists.txt (1 hunks)
  • src/Server/Systems/Network/MainServerNetwork.cpp (2 hunks)
  • src/Server/Systems/Network/ServerNetwork.cpp (7 hunks)
  • src/Server/Systems/Network/ServerNetwork.hpp (1 hunks)
  • src/Server/Systems/WaveSystem.cpp (1 hunks)
  • src/Server/Systems/WaveSystem.hpp (1 hunks)
  • src/ServerArgsHandling.hpp (1 hunks)
  • src/init.cpp (4 hunks)
  • src/init.hpp (1 hunks)
  • src/main_server.cpp (2 hunks)
Files skipped from review due to trivial changes (10)
  • src/CMakeLists.txt
  • src/Client/Systems/CMakeLists.txt
  • src/Client/Systems/ClientSystems.cpp
  • src/Client/Systems/Menus/CMakeLists.txt
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp
  • src/Game/CMakeLists.txt
  • src/Game/PhysicSystems.cpp
  • src/Server/CMakeLists.txt
  • src/ServerArgsHandling.hpp
  • src/init.hpp
Additional comments: 163
src/Client/Systems/Menus/CreateLobby/CMakeLists.txt (1)
  • 1-13: The CMakeLists.txt file looks good. It correctly specifies the minimum required CMake version, includes the current source directory as a private include directory for the ${PROJECT_NAME_CLIENT} target, and adds the "CreateLobbySystems.cpp" source file to the ${PROJECT_NAME_CLIENT} target. Ensure that the file "CreateLobbySystems.cpp" exists in the current source directory.
src/Client/Systems/Events/EventsSystems.hpp (1)
  • 16-22: The new function handleEndGameEvent has been added correctly. However, ensure that this function is implemented somewhere in the codebase and is being called appropriately. Also, make sure that the function's parameters are being used, as they are currently marked as unused.
void handleEndGameEvent(std::size_t /*unused*/, std::size_t /*unused*/);
docs/developer-guide/client/graphic/text.md (1)
  • 27-31: The function signature has been changed from setCurrentText to setText. Ensure that all references to this function in the codebase have been updated to reflect this change. Also, verify that the new function setText is being used correctly with the new parameter type const std::string &text.
src/Client/Systems/Menus/Menu/Menu.hpp (3)
  • 15-19: The new enum value SPRITE has been added to ObjectType. Ensure that all code that uses this enum is updated to handle this new value.

  • 61-64: The function initMenuEntity now returns a std::size_t value and takes a nlohmann::json parameter instead of a reference. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 65-65: A new function initMenuSceneEntity has been added. Ensure that it is being used correctly and that its implementation is correct.

src/Client/Systems/Network/ClientNetwork.hpp (1)
  • 15-22: The new functions receiveMissileDeath, receiveConnectLobbyResp, and receiveEndGame have been added to the Systems namespace. Ensure that these functions are implemented in the corresponding source file (ClientNetwork.cpp) and that they are used correctly throughout the codebase.
src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.hpp (1)
  • 20-28: The LobbyStatus struct now includes two static member variables: pageNbr and pageMax. This is a significant change as static member variables share the same value across all instances of the struct. Ensure that this is the intended behavior. Also, the constructor LobbyStatus now has default parameter values for ip and port. Make sure that all instances of LobbyStatus are created with these default values in mind.
src/Client/Systems/Graphic/GraphicSystems.cpp (3)
  • 21-44: The debugCollisionRect function is only compiled when NDEBUG is not defined. This is a common practice for debug functions, but make sure that NDEBUG is defined in production builds to avoid unnecessary overhead.

  • 24-24: The std::lock_guard is used to ensure thread safety when accessing the Registry singleton. This is a good practice, but make sure that all other accesses to the Registry are also protected by a mutex to avoid data races.

  • 46-61: The getGraphicsSystems function has been modified to include the debugCollisionRect function in the audioSystems vector. This is fine as long as the debugCollisionRect function signature matches the expected function signature for the vector. Also, ensure that the debugCollisionRect function is only added in debug builds.

src/Client/Systems/Parallax/Parallax.cpp (1)
  • 42-48: The std::lock_guard is used correctly to prevent data races when accessing the Registry singleton. The change from JsonType::PARALLAX to JsonType::DEFAULT_PARALLAX is consistent with the PR summary. Ensure that the JsonType::DEFAULT_PARALLAX is correctly defined and the corresponding JSON data is correctly formatted.
src/Client/Systems/Menus/Menu/ButtonCallbacks.hpp (1)
  • 14-74: The file introduces a new namespace "Menu::Callback" and defines an enum "CallbackType" with several callback types. It also declares several callback functions and maps them to the corresponding callback types in the "callbacks" unordered map. The changes look good.
scripts/compil.sh (1)
  • 12-16: The script now supports a --no-parallel argument to disable parallel building. This is a good addition for systems with limited resources or for debugging purposes. However, the script should document this new argument in a comment or usage message.
src/Game/PhysicSystems.hpp (1)
  • 1-8: The function addPhysicsToEntity is declared but not defined in this file. Ensure that it is defined in the corresponding source file (PhysicSystems.cpp), and that the definition matches the declaration. Also, make sure that the function is being used somewhere in the codebase, otherwise it's just dead code.
namespace Systems {
    void addPhysicsToEntity(nlohmann::json jsonObject, const Types::Position &originPos) {
        // function implementation goes here
    }
}
src/Game/WaveSystemsAll.hpp (1)
  • 1-6: The new getWaveSystems function in the Systems namespace returns a vector of std::function objects. Ensure that the function is implemented correctly and that the returned std::function objects are used properly throughout the codebase. Also, make sure that the std::function objects are properly initialized before use to avoid undefined behavior.
src/Client/Systems/Events/EventsSystems.cpp (6)
  • 18-21: The inclusion of new headers should be verified to ensure they are necessary and not redundant. Also, ensure that the new header init.hpp is available in the project.

  • 120-126: The changes in this hunk seem to be related to the refactoring of the playerShootBullet function. Ensure that the changes do not affect the functionality of the function.

  • 137-141: The changes in this hunk seem to be related to the refactoring of the playerShootBullet function. Ensure that the changes do not affect the functionality of the function.

  • 166-174: The changes in this hunk seem to be related to the refactoring of the playerMovement function. Ensure that the changes do not affect the functionality of the function.

  • 224-265: The addition of isGameWin and modifEndGameText functions seems to be a good move towards modularity. However, ensure that these functions are called appropriately in the codebase.

  • 267-289: The changes in the handleEndGameEvent function seem to be related to the addition of the isGameWin and modifEndGameText functions. Ensure that the changes do not affect the functionality of the function.

src/Game/ResourcesManager.hpp (2)
  • 10-20: The JsonType enum class has been updated. Ensure that all references to this enum throughout the codebase have been updated to match the new values.

  • 24-34: The paths vector has been updated. Ensure that all JSON files exist at the specified paths and that all references to this vector throughout the codebase have been updated to match the new paths.

src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.cpp (6)
  • 13-13: Ensure that the new include file "B-luga/Maths/Maths.hpp" exists and is accessible from this location.

  • 25-26: The static variables pageNbr and pageMax are introduced. Make sure they are used correctly and their values are managed properly throughout the code.

  • 71-103: The new function initLobbyRow is added. Ensure that it is called correctly and that it behaves as expected.

  • 129-135: The new function updateMaxPage is added. Ensure that it is called correctly and that it behaves as expected.

  • 137-170: The new function updateLobbyRow is added. Ensure that it is called correctly and that it behaves as expected.

  • 172-175: The function getLobbySystems is updated to include initLobbyRow and updateLobbyRow. Verify that it behaves as expected.

src/Nitwork/MessageTypes.h (2)
  • 14-26: The addition of new enemy types is well done. Ensure that these new types are handled correctly in the rest of the codebase.

  • 28-34: The addition of new missile types is also well done. As with the enemy types, ensure that these new missile types are handled correctly in the rest of the codebase.

src/Nitwork/ANitwork.cpp (2)
  • 247-251: The error message in the catch block has been updated to include the function name. This is a good practice as it helps in identifying the source of the error more easily.

  • 392-404: A new function deletePacketFromEndPoints has been added. This function removes packets from the _outputQueue that have the same IP address and port as the specified endpoint. This could be useful in scenarios where you want to cancel all pending packets to a specific endpoint. However, ensure that this function is used judiciously as it could lead to loss of important data if not handled properly.

src/Game/GameCustomTypes.cpp (4)
  • 9-11: The inclusion of "B-luga-graphics/Raylib/Raylib.hpp" is conditional on the CLIENT macro. Ensure that this macro is defined in the appropriate build configurations.

  • 15-16: The static member variables _missileNb and _mutex are added to the Types::Missiles class. Ensure that these variables are used correctly and thread-safely throughout the codebase.

  • 20-25: The prepareNextWave() function is added to the Types::WaveInfos class. Ensure that this function is called at the appropriate times in the game logic.

  • 27-59: The setWaitingForNextWave(bool value) function is added to the Types::WaveInfos class. This function contains code that interacts with the Registry and Raylib classes when CLIENT is defined. Ensure that this function is used correctly and that the CLIENT macro is defined in the appropriate build configurations.

src/Game/GameCustomTypes.hpp (5)
  • 8-14: The inclusion of new headers and the use of a mutex for thread safety are good practices. Ensure that the new header file "B-luga-physics/ECSCustomTypes.hpp" exists and is accessible from the current file's location.

  • 27-54: The Missiles struct has been updated to use a mutex for thread safety in setMissileNb and getMissileNb functions. This is a good practice when dealing with shared data in a multithreaded context.

  • 60-70: The new EnemyAttack struct has been added. Ensure that all its member variables are used appropriately in the codebase.

  • 27-90: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [72-129]

The Enemy struct has been significantly updated. The use of a mutex for thread safety in setEnemyNb and getEnemyNb functions is a good practice. The addition of the EnemyAttack member variable attack and the getAttack function are significant changes. Ensure that they are used correctly in the codebase.

  • 131-219: The new WaveInfos class has been added. It includes several member variables and functions. Ensure that all of them are used appropriately in the codebase. The use of the singleton pattern in getInstance is a common practice. The deletion of copy and move constructors and assignment operators is a good practice to prevent unwanted copying or moving of the singleton instance.
src/Nitwork/Nitwork.h (5)
  • 18-23: The maximum number of clients for the main server has been increased from 30 to 300. This is a significant increase and could have implications for server performance and resource usage. Please ensure that the server is capable of handling this increased load.

  • 34-40: The magic number for a new missile has been changed from MAGICK_NEW_BULLET to MAGICK_NEW_MISSILE. Ensure that all references to this magic number in the codebase have been updated to reflect this change.

  • 45-52: New action types have been added for missile death, connecting to a lobby, disconnecting from a lobby, and ending the game. Ensure that these new actions are handled correctly in the relevant parts of the codebase.

  • 62-88: The enum n_actionType_t has been updated to include the new action types. Ensure that all switch statements or other code that uses this enum has been updated to handle the new values.

  • 407-427: Several new message and packet structures have been added to support the new actions. Ensure that these structures are used correctly in the codebase and that all necessary fields are initialized properly.

src/Nitwork/ANitwork.hpp (3)
  • 9-19: The new include <type_traits> is used for the std::is_same_v function in the debug log statement. Ensure that this include is necessary in other parts of the code as well, to avoid unnecessary includes.

  • 64-71: The new debug log statement is conditional on the type of T not being struct packetListLobby_s. This is a good use of if constexpr and std::is_same_v to perform compile-time checks. However, ensure that the log level Logger::fatal is appropriate for this situation.

  • 122-122: The new function deletePacketFromEndPoints is added to the NitworkServer class. Ensure that this function is implemented in the corresponding .cpp file and that it is used correctly throughout the codebase.

src/Client/Systems/Menus/Menu/Menu.cpp (10)
  • 24-49: The function initSpriteFromJson has been refactored to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the returned value.

  • 51-71: The function initFromSprite has been refactored and no longer returns a value. Ensure that all calls to this function throughout the codebase have been updated to reflect this change.

  • 73-104: The function initInputBoxText has been added. This function initializes the text for an input box. Ensure that this function is being called correctly where needed.

  • 21-117: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [106-127]

The function initInputBox has been refactored to call the new initInputBoxText function. Ensure that all calls to this function throughout the codebase have been updated to reflect this change.

  • 129-144: The function initButtonFromSprite has been refactored to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the returned value.

  • 121-149: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [146-163]

The function initButtonFromRectangle has been refactored to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the returned value.

  • 166-192: The function initText has been added. This function initializes a text entity. Ensure that this function is being called correctly where needed.

  • 194-201: The function initButton has been refactored to call either initButtonFromSprite or initButtonFromRectangle based on whether a sprite path exists in the JSON data. Ensure that all calls to this function throughout the codebase have been updated to reflect this change.

  • 203-219: The function initMenuEntity has been refactored to take a single nlohmann::json parameter instead of separate parameters. Ensure that all calls to this function throughout the codebase have been updated to reflect this change.

  • 221-233: The function initMenuSceneEntity has been added. This function initializes menu entities from an array of JSON data. Ensure that this function is being called correctly where needed.

src/Game/GameSystems.cpp (8)
  • 1-12: The inclusion of new headers CreateMissiles.hpp and WaveSystemsAll.hpp is noted. Ensure that these files exist and are correctly referenced.

  • 34-40: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [17-81]

The initPlayer function has been modified to use the Types::Health struct instead of struct health_s. This change should be verified across the codebase to ensure consistency.

  • 83-94: The sendMissileDeath function has been added. This function sends a missile death message. Ensure that this function is correctly implemented and used.

  • 75-100: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [96-108]

The sendEnemyDeath function has been modified to remove a log statement. This change should not affect the functionality of the function.

  • 110-120: The sendLifeUpdateToServer function has been modified to use the struct health_s struct instead of Types::Health. This change should be verified across the codebase to ensure consistency.

  • 105-126: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [122-134]

The sendPlayerDeathToServer function has been modified to remove log statements. This change should not affect the functionality of the function.

  • 139-145: The GamePlugin::initPlugin function has been updated to include bullet systems and wave systems. Ensure that these systems are correctly implemented and used.

  • 156-166: The getSystems function has been updated to include bullet systems and wave systems. Ensure that these systems are correctly implemented and used.

src/Server/Systems/CMakeLists.txt (1)
  • 1-15: The changes to the CMakeLists.txt file look good. The new source file "WaveSystem.cpp" is correctly added to the target "${PROJECT_NAME_SERVER}" and the current source directory is included in the target's private include directories. Ensure that "WaveSystem.cpp" exists and is in the correct location relative to this CMakeLists.txt file.
src/Server/Systems/Network/ServerNetwork.hpp (1)
  • 16-19: The new function handleClientMissileDeath has been added correctly. Ensure that the function is implemented somewhere in the codebase and that it is being called appropriately. Also, verify that the function's parameters are being used correctly within the function.
src/Nitwork/NitworkMainServer.hpp (3)
  • 82-96: New methods setIpOfMainServer and getAvailableIps have been added. Ensure that these methods are used correctly throughout the codebase.

  • 151-154: A new member variable _ip has been added. Ensure that this variable is initialized properly and used correctly.

  • 202-214: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [157-214]

The action types have been renamed. Ensure that these new names are reflected throughout the codebase.

- CONNECT_MAIN_SERVER
+ NITWORK_CONNECT_MAIN_SERVER
- LIST_LOBBY
+ NITWORK_LIST_LOBBY
- CREATE_LOBBY
+ NITWORK_CREATE_LOBBY
- INFO_LOBBY
+ NITWORK_INFO_LOBBY
- CONNECT_MAIN_SERVER_RESP
+ NITWORK_CONNECT_MAIN_SERVER_RESP
src/Client/Systems/Network/ClientNetwork.cpp (11)
  • 21-34: Ensure that the Types::Health component is correctly initialized and updated throughout the codebase. Also, consider adding error handling for the case when ids is empty.

  • 39-45: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [39-55]

Similar to the previous comment, ensure that the Types::Health and Types::Enemy components are correctly initialized and updated throughout the codebase. Also, consider adding error handling for the case when ids is empty.

  • 58-90: The handleStartWave function has been updated to use Types::WaveInfos instead of Types::Enemy. Ensure that this change is reflected throughout the codebase and that Types::WaveInfos is correctly initialized and updated.

  • 99-104: The receiveNewEnemy function has been updated to use Types::Health instead of struct health_s. Ensure that this change is reflected throughout the codebase and that Types::Health is correctly initialized and updated.

  • 110-118: The createNewPlayer function has been updated to use Types::Health instead of struct health_s. Ensure that this change is reflected throughout the codebase and that Types::Health is correctly initialized and updated.

  • 176-183: The sendInitPacket function has been added. Ensure that it is correctly called and that its effects are handled properly throughout the codebase.

  • 263-275: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [253-273]

The receiveNewBullet function has been updated to use Types::Missiles and Types::Health instead of struct health_s. Ensure that this change is reflected throughout the codebase and that Types::Missiles and Types::Health are correctly initialized and updated.

  • 376-390: The receiveConnectLobbyResp function has been added. Ensure that it is correctly called and that its effects are handled properly throughout the codebase.

  • 392-400: The quitGame function has been added. Ensure that it is correctly called and that its effects are handled properly throughout the codebase.

  • 402-408: The receiveEndGame function has been added. Ensure that it is correctly called and that its effects are handled properly throughout the codebase.

  • 410-413: The list of network systems returned by getNetworkSystems has been updated to include the new functions. Ensure that this change is reflected throughout the codebase.

src/Game/WaveSystemsAll.cpp (1)
  • 225-228: The getWaveSystems function is well implemented. It's a good practice to return a list of functions to be executed.
src/Game/CreateMissiles.cpp (11)
  • 11-20: The new includes seem to be necessary for the new functions and changes in the file. Ensure that these headers are available in the project and that they are correctly referenced.

  • 38-58: The new functions getMissileTypeFromId and getMissileIdFromType are used to map between missile types and their string identifiers. This is a good practice as it allows for easier management of missile types.

  • 60-70: The playBulletSound function has been updated to use getMissileIdFromType instead of getMissileId. This is a good change as it makes the function more flexible and easier to use.

  • 81-94: The addSpriteRectsForBullet function has been updated to use Json::isDataExist instead of json.isDataExist. This is a good change as it makes the function more robust and less prone to errors.

  • 100-133: The new function createPlayerMissile creates a new player missile entity and adds it to the registry. This function seems to be well implemented and follows good practices.

  • 135-164: The new function createEnemyMissile creates a new enemy missile entity and adds it to the registry. This function seems to be well implemented and follows good practices.

  • 166-190: The new function launchMissileInLine launches a series of missiles in a line. This function seems to be well implemented and follows good practices.

  • 192-221: The new function launchMissileInCircle launches a series of missiles in a circle. This function seems to be well implemented and follows good practices.

  • 223-231: The new function launchEnemyMissile launches an enemy missile based on the enemy's attack emitter type. This function seems to be well implemented and follows good practices.

  • 233-252: The new function updateEnemiesAttacks updates the attacks of all enemies. This function seems to be well implemented and follows good practices.

  • 254-257: The new function getBulletsSystems returns a vector of bullet system functions. This function seems to be well implemented and follows good practices.

src/Server/Systems/Network/MainServerNetwork.cpp (1)
  • 59-60: The setIpOfMainServer function is being called with the IP of the owner of the lobby. Ensure that this is the intended behavior, as it could potentially change the IP of the main server every time a new lobby is created.

60:
The createLobby function has been updated to include additional parameters for maximum number of players, lobby name, and game type. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

src/Nitwork/NitworkClient.hpp (7)
  • 37-40: The return type of connectMainServer has been changed from void to bool. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 37-43: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [42-47]

A new function connectLobby has been added. Ensure that it is being used correctly in the codebase.

  • 52-52: A new function disconnectLobby has been added. Ensure that it is being used correctly in the codebase.

  • 58-58: The function addInitMsg has been moved from private to public. Ensure that this change does not violate encapsulation principles and that the function is not being misused in the public scope.

  • 103-104: A new function addMissileDeathMsg has been added. Ensure that it is being used correctly in the codebase.

  • 142-147: A new function addConnectLobbyMsg has been added. Ensure that it is being used correctly in the codebase.

  • 226-243: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [226-391]

The action types and corresponding handlers have been modified to use new message types. A new action type NITWORK_END_GAME and its corresponding handler have been added. Ensure that these changes are consistent with the rest of the codebase and that the new action type is being handled correctly.

src/main_server.cpp (3)
  • 14-20: The signalHandler() function has been updated to stop the SceneManager before setting isRunning to false. This is a good practice as it ensures that the SceneManager is stopped before the program is terminated.

  • 22-30: The displayAvailableIps() function retrieves and logs the available IP addresses. This can be useful for debugging and monitoring purposes.

  • 37-43: The displayAvailableIps() function is called after starting the main server. This is a good place to call this function as it ensures that the IP addresses are logged every time the server is started.

src/Client/Systems/Menus/Menu/MenuSystems.cpp (13)
  • 6-41: The setAllInputBoxFalse function iterates over all entities with InputBox and AnimRect components and sets the selected field of InputBox to false and changes the RectListType of AnimRect to DEFAULT_RECT. This function seems to be used to deselect all input boxes.

  • 45-62: The setInputBoxSelected function sets the selected field of the InputBox component of the entity with the given id to true and changes the RectListType of its AnimRect component to SELECTED. This function seems to be used to select a specific input box.

  • 77-89: The checkTextInput function checks if any InputBox is selected and if so, it inserts text into it. It seems to be used to handle text input into selected input boxes.

  • 94-101: The manageInputBox function checks if the left mouse button is pressed and if so, it checks if a click occurred inside an input box and selects it. This function seems to be used to handle mouse clicks on input boxes.

  • 112-132: The checkInputDeletion function checks if the backspace key is pressed and if so, it deletes a character from the text of the selected input box. This function seems to be used to handle text deletion in selected input boxes.

  • 135-154: The hoverInputBox function checks if the mouse is hovering over an input box and if so, it changes the RectListType of its AnimRect component to HOVER. This function seems to be used to handle mouse hover events on input boxes.

  • 156-168: The manageClick function checks if the left mouse button is pressed or released on a button and if so, it changes the RectListType of its AnimRect component accordingly and calls the button's callback function if the button is released. This function seems to be used to handle mouse clicks on buttons.

  • 170-176: The pressButton function checks if the mouse is inside a button and if so, it calls the manageClick function for that button. This function seems to be used to handle mouse events on buttons.

  • 189-206: The initMenu function initializes the menu scene by loading the menu data from a JSON file and creating the menu entities. It also removes the initMenu system from the system manager after the menu is initialized. This function seems to be used to initialize the menu scene.

  • 208-225: The quitScene function checks if the escape key is pressed and if so, it changes the current scene or stops the game depending on the current scene. This function seems to be used to handle the escape key press event.

  • 227-249: The preloadTexture function preloads textures for the enemies, default parallax, and default player from JSON files. This function seems to be used to preload textures for the game.

  • 251-264: The initSceneGame function initializes the game scene by loading the game HUD from a JSON file, preloading textures, and initializing the parallax. It also removes the initSceneGame system from the system manager after the game scene is initialized. This function seems to be used to initialize the game scene.

  • 271-277: The getMenuSystems function returns a vector of menu systems. This function seems to be used to get all the menu systems.

src/Nitwork/NitworkMainServer.cpp (7)
  • 5-20: The inclusion of platform-specific headers and the order of includes is correct. The #ifdef _WIN32 directive is used to include Windows-specific headers, and the #else directive is used for Unix/Linux-specific headers. The order of includes is also correct, with platform-specific includes coming before project-specific includes.

  • 62-68: The renaming of the CONNECT_MAIN_SERVER_RESP constant to NITWORK_CONNECT_MAIN_SERVER_RESP is a good practice as it makes the constant more specific and reduces the chance of name collisions.

  • 75-83: The modification of the handleInitMsg function to send a response when a client is already connected is a good practice. It provides feedback to the client and can help with debugging connection issues.

  • 155-161: The renaming of the LIST_LOBBY constant to NITWORK_LIST_LOBBY is a good practice as it makes the constant more specific and reduces the chance of name collisions.

  • 174-201: The modification of the forkProcessAndCreateLobby function to pass the name parameter as a command-line argument to the r-type_server process is a good practice. It allows the r-type_server process to have access to the name of the lobby, which can be useful for logging or other purposes.

  • 237-241: The addition of the createLobby function is a good practice. It encapsulates the logic for creating a lobby, making the code more modular and easier to maintain.

  • 269-291: The addition of the setIpOfMainServer and getAvailableIps functions is a good practice. These functions provide a way to set and retrieve the IP address of the main server, which can be useful for networking purposes.

src/Nitwork/NitworkServer.hpp (6)
  • 54-75: The new functions addStarWaveMessage, addConnectLobbyRespMsg, addMissileDeathMsg, and addEndGameMsg are well-documented and seem to be logically correct. However, ensure that these functions are being called correctly in the codebase and that the messages they create are being handled properly.

  • 140-151: The new functions addMissileDeathMsg and addEndGameMsg are well-documented and seem to be logically correct. However, ensure that these functions are being called correctly in the codebase and that the messages they create are being handled properly.

  • 210-248: The new function handleConnectLobbyMsg is well-documented and seems to be logically correct. However, ensure that this function is being called correctly in the codebase and that the messages it handles are being created properly.

  • 258-267: The new member variables _playersReady and _isGameStarted are well-documented and seem to be logically correct. However, ensure that these variables are being used correctly in the codebase.

  • 273-351: The _actionsHandlers map has been updated to handle the new action types. Ensure that these new handlers are functioning correctly and that the messages they handle are being created properly.

  • 355-411: The _actionToSendHandlers map has been updated to handle the new action types. Ensure that these new handlers are functioning correctly and that the messages they handle are being created properly.

src/Server/Systems/Network/ServerNetwork.cpp (2)
  • 75-96: The function receiveNewBulletMsg has been updated to not take a reference to boost::asio::ip::udp::endpoint. Ensure that this change does not affect the rest of the codebase.

  • 161-192: A new function handleClientMissileDeath has been added. Ensure that this function is being called appropriately in the rest of the codebase.

src/init.cpp (4)
  • 14-14: Ensure that the new header file "CreateLobbySystems.hpp" exists and is in the correct path. Also, verify that it contains the necessary definitions and declarations needed in this file.

  • 46-46: Ensure that the function getCreateLobbySystems exists in the Systems::CreateLobby namespace and returns the correct type. Also, verify that all dependencies of this function are properly handled.

  • 64-66: Ensure that the SystemManagers::NETWORK_CREATE_LOBBY enum value is defined and correctly used in the enumListTosizet function. Also, verify that the createLobby vector is used correctly in the subsequent code.

  • 74-74: Ensure that the sceneManager.setScenes() function can handle the new createLobby scene correctly. Also, verify that the order of the scenes in the list is correct according to the application logic.

src/Nitwork/NitworkClient.cpp (9)
  • 8-10: This preprocessor directive is used to suppress warnings about unsafe functions in Windows. It's generally not recommended to suppress warnings as they can indicate potential issues in your code. However, if you're sure that the usage is safe and the warnings are not helpful, this is acceptable.

  • 125-134: The connectMainServer function now returns a bool indicating success or failure. This is a good practice as it allows the caller to handle the failure case appropriately. The function also now catches exceptions, logs an error message, and returns false in case of an exception. This is a good way to handle exceptions and prevent them from propagating up the call stack.

  • 144-160: The disconnectLobby function is a new addition. It appears to be correctly implemented, creating a packetDisconnectLobby_s structure and adding it to the send queue. The use of a std::lock_guard to protect the _receivedPacketsIdsMutex is a good practice for thread safety.

  • 166-172: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [162-172]

The addInitMsg function has been renamed to addConnectLobbyMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 331-350: The addMissileDeathMsg function is a new addition. It appears to be correctly implemented, creating a packetMissileDeath_s structure and adding it to the send queue. The use of a std::lock_guard to protect the _receivedPacketsIdsMutex is a good practice for thread safety.

  • 352-362: The addListLobbyMsg function has been renamed to addRequestListLobbyMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 377-383: The addCreateLobbyMsg function has been renamed to addCreateLobbyRequestMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 407-413: The addConnectMainServerMsg function has been renamed to addConnectMainServerRequestMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 421-436: The addConnectLobbyMsg function is a new addition. It appears to be correctly implemented, creating a packetConnectLobby_s structure and adding it to the send queue. The use of a std::lock_guard to protect the _receivedPacketsIdsMutex is a good practice for thread safety.

docs/network/rfc/RFC.md (6)
  • 36-47: The new actions CONNECT_LOBBY, NEW_MISSILE, MISSILE_DEATH, and DISCONNECT_LOBBY have been added to the Client -> Lobby Server section. Ensure that these actions are implemented and handled correctly in the client and lobby server code.

  • 49-63: The new actions CONNECT_LOBBY_RESP and END_GAME have been added to the Lobby Server -> Client section. Ensure that these actions are implemented and handled correctly in the lobby server and client code.

  • 200-200: The magick value for the CONNECT_MAIN_SERVER action has been updated to 19. Ensure that this change is reflected in the client and server code where this action is used.

  • 230-230: The magick value for the CREATE_LOBBY action has been updated to 23. Ensure that this change is reflected in the client and server code where this action is used.

  • 557-606: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [297-598]

The descriptions and field details for the new actions CONNECT_LOBBY, NEW_MISSILE, MISSILE_DEATH, and DISCONNECT_LOBBY have been added. Ensure that these details are correctly implemented in the client and lobby server code.

  • 1094-1143: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [706-1135]

The descriptions and field details for the new actions CONNECT_LOBBY_RESP and END_GAME have been added. Ensure that these details are correctly implemented in the lobby server and client code.

src/Nitwork/NitworkServer.cpp (8)
  • 202-206: This code checks if the game has already started or if the maximum number of players has been reached, and if so, it returns early. This is a good use of guard clauses to prevent further execution when certain conditions are met.

  • 251-268: This code checks if the game has already started or if the client is not already connected, and if so, it returns early. This is another good use of guard clauses. However, the comment on line 267 is unclear and should be improved for better readability.

  • 301-320: This code handles a disconnect message from a client. It checks if the message's magick number matches the expected value, and if not, it logs an error and returns early. This is a good use of guard clauses. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

  • 325-337: This code creates a connection lobby response message and adds it to the packet queue. It uses a lock to ensure thread safety, which is good. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

  • 381-388: This code creates a start wave message and adds it to the packet queue. It uses a lock to ensure thread safety, which is good. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

  • 444-455: This code creates a new bullet message and sends it to all clients. It uses a lock to ensure thread safety, which is good. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

  • 523-541: This code creates a missile death message and adds it to the packet queue. It uses a lock to ensure thread safety, which is good. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

  • 543-554: This code creates an end game message and sends it to all clients. It uses a lock to ensure thread safety, which is good. However, the code could be improved by abstracting some of the logic into separate functions for better readability and maintainability.

src/Client/Systems/ClientSystems.hpp Show resolved Hide resolved
scripts/compil.sh Show resolved Hide resolved
src/Client/Systems/Menus/Menu/ButtonCallbacks.hpp Outdated Show resolved Hide resolved
src/Client/Systems/CustomTypes/AnimRect.hpp Show resolved Hide resolved
src/Server/Systems/Network/ServerNetwork.cpp Show resolved Hide resolved
src/Server/Systems/Network/ServerNetwork.cpp Show resolved Hide resolved
src/init.cpp Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Outdated Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Outdated Show resolved Hide resolved
Copy link
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.

Review Status

Actionable comments generated: 26

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 75b39ff and 2b00212.
Files ignored due to filter (3)
  • assets/Json/menu.json
  • assets/Json/playerData.json
  • assets/Menu/loading.gif
Files selected for processing (27)
  • src/Client/Systems/Events/EventsSystems.cpp (6 hunks)
  • src/Client/Systems/Menus/CreateLobby/CreateLobbySystems.cpp (1 hunks)
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp (1 hunks)
  • src/Client/Systems/Menus/Menu/Menu.cpp (3 hunks)
  • src/Client/Systems/Menus/Menu/MenuSystems.cpp (1 hunks)
  • src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.cpp (2 hunks)
  • src/Client/Systems/Network/ClientNetwork.cpp (12 hunks)
  • src/Game/CreateMissiles.cpp (5 hunks)
  • src/Game/GameCustomTypes.cpp (1 hunks)
  • src/Game/GameCustomTypes.hpp (4 hunks)
  • src/Game/GameSystems.cpp (6 hunks)
  • src/Game/PhysicSystems.cpp (1 hunks)
  • src/Game/ResourcesManager.hpp (1 hunks)
  • src/Game/WaveSystemsAll.cpp (1 hunks)
  • src/Nitwork/ANitwork.cpp (2 hunks)
  • src/Nitwork/ANitwork.hpp (3 hunks)
  • src/Nitwork/Nitwork.h (8 hunks)
  • src/Nitwork/NitworkClient.cpp (14 hunks)
  • src/Nitwork/NitworkMainServer.cpp (8 hunks)
  • src/Nitwork/NitworkMainServer.hpp (7 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
  • src/Nitwork/NitworkServer.hpp (6 hunks)
  • src/Server/Systems/Network/ServerNetwork.cpp (7 hunks)
  • src/Server/Systems/WaveSystem.cpp (1 hunks)
  • src/Server/Systems/WaveSystem.hpp (1 hunks)
  • src/init.cpp (4 hunks)
  • src/init.hpp (1 hunks)
Files skipped from review due to trivial changes (4)
  • src/Game/PhysicSystems.cpp
  • src/Nitwork/ANitwork.hpp
  • src/Nitwork/NitworkServer.cpp
  • src/init.hpp
Additional comments: 118
src/Game/WaveSystemsAll.cpp (6)
  • 1-6: The file header comment is still good practice for providing a brief description of the file's purpose. However, it could be more descriptive about the specific functionality provided by the Wave Systems implementation.

  • 34-44: The enemiesTypes map is still a good way to associate enemy types with their string identifiers. However, consider using an enum class for enemy_type_e to provide stronger type safety and better encapsulation.

  • 46-64: The getEnemyId and getEnemyType functions are still well implemented with error handling. However, consider using std::optional instead of throwing exceptions for a more modern and safer approach.

  • 80-112: The initEnemyWeapon function is still well implemented. However, consider breaking it down into smaller functions for better readability and maintainability.

  • 170-193: The initWave function is still well implemented. However, consider breaking it down into smaller functions for better readability and maintainability.

  • 195-224: The updateWave function is still well implemented. However, it's quite long and does a lot of things. Consider breaking it down into smaller functions for better readability and maintainability.

src/Nitwork/ANitwork.cpp (2)
  • 247-254: The error handling in the startInputHandler function has been improved to provide more context in the log message when an exception is thrown. This is a good practice as it helps in debugging and understanding the cause of the error.

  • 393-405: A new function deletePacketFromEndPoints has been added. This function removes packets from the _outputQueue that have a matching endpoint address and port. This could be useful in scenarios where you want to cancel all packets destined for a specific endpoint. However, ensure that this function is used judiciously as it could lead to data loss if not handled properly.

src/Game/ResourcesManager.hpp (2)
  • 10-20: The JsonType enum has been modified. Ensure that all references to the removed enum values (DEFAULT_ENEMY and TERMINATORBOSS) have been updated or removed from the codebase. Also, verify that the newly added enum values (DEFAULT_PARALLAX, WAVE, MENU, SELECT_LOBBY, CREATE_LOBBY, HUD, and ENEMIES) are being used correctly.

  • 24-34: The ResourcesManagerValues::paths vector has been updated to reflect the changes in the JsonType enum. Ensure that the paths are correct and the corresponding JSON files exist at the specified locations.

src/Game/GameCustomTypes.hpp (5)
  • 8-14: The inclusion of the new header file and the start of the namespace are fine.

  • 27-54: The Missiles struct has been updated to include a mutex and static variables. This is a good practice for thread safety when accessing shared data.

  • 60-70: The new EnemyAttack struct has been added. Ensure that all its member variables are used appropriately in the codebase.

  • 27-90: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [72-129]

The Enemy struct has been updated to include a mutex, static variables, and an instance of EnemyAttack. This is a good practice for thread safety when accessing shared data. The getAttack function has been added to access the attack member.

  • 131-228: The new WaveInfos class has been added. It includes a singleton pattern implementation, which is a good practice for ensuring only one instance of the class exists. It also includes several member functions and variables. Ensure that all these are used appropriately in the codebase.
src/Nitwork/Nitwork.h (8)
  • 18-22: The maximum number of clients for the main server has been increased from 30 to 300. Ensure that the server can handle this increased load.

  • 34-40: The constant for a new bullet has been renamed from MAGICK_NEW_BULLET to MAGICK_NEW_MISSILE. Make sure to update all references to this constant throughout the codebase.

  • 45-54: Several new action types have been added. Ensure that these new actions are handled correctly in the rest of the codebase.

  • 63-91: The msgStartWave_s struct now includes a waveId field instead of enemyNb. Make sure to update all references to this struct throughout the codebase.

  • 128-134: The msgNewBullet_s struct now includes an id field and a life field in addition to the existing fields. Make sure to update all references to this struct throughout the codebase.

  • 190-198: New structs and packets have been added for the new action types. Ensure that these new structs and packets are used correctly in the rest of the codebase.

  • 266-284: The msgPlayerDeath_s struct now includes a msg field. Make sure to update all references to this struct throughout the codebase.

  • 380-442: Several new structs and packets have been added for the new action types. Ensure that these new structs and packets are used correctly in the rest of the codebase.

src/Game/GameCustomTypes.cpp (2)
  • 15-16: The static member variable _missileNb and _mutex are added to the Types::Missiles class. Ensure that these variables are used correctly and thread-safely throughout the codebase.

  • 29-62: The #ifdef CLIENT preprocessor directive is used to conditionally compile code. Ensure that this code is tested in both the CLIENT and non-CLIENT cases to verify that it behaves as expected.

src/Client/Systems/Menus/SelectLobby/SelectLobbySystems.cpp (7)
  • 10-21: The new includes are added correctly. Ensure that the paths are correct and the files exist.

  • 32-47: The initSelectLoby function has been modified to check for the SELECT_LOBBY scene and initialize the parallax effect and menu entities. Ensure that the SELECT_LOBBY scene and the Parallax::initParalax() function exist and work as expected. Also, make sure that the JSON data is correctly formatted and contains the necessary information.

  • 72-106: The initLobbyRow function has been added. It initializes the lobby rows by creating menu entities and setting their positions. Ensure that the JSON data is correctly formatted and contains the necessary information. Also, make sure that the ::Menu::MenuBuilder::getInstance().initMenuEntity(lobbyBox) function works as expected.

  • 108-130: The updateBox function has been added. It updates the lobby boxes with the lobby information. Ensure that the lobby information is correctly formatted and contains the necessary information. Also, make sure that the Raylib::Text::fromText(text_t) function works as expected.

  • 132-138: The updateMaxPage function has been added. It updates the maximum page number based on the number of lobbies. Ensure that the calculation is correct.

  • 140-173: The updateLobbyRow function has been modified to update the lobby boxes and the maximum page number, and to handle pagination. Ensure that the pagination works as expected.

  • 175-178: The getLobbySystems function has been updated to include the new functions. Ensure that the functions are correctly added to the vector.

src/Client/Systems/Events/EventsSystems.cpp (4)
  • 18-24: The inclusion of "init.hpp" seems to be a new addition. Ensure that this header file exists and is in the correct location.

  • 120-126: The usage of struct health_s has been replaced with Types::Health. Ensure that this type exists and is correctly defined.

  • 221-269: The new functions isGameWin and modifEndGameText have been added. Ensure that these functions are being called correctly and that they behave as expected.

  • 271-301: The handleEndGameEvent function has been modified. Ensure that the new logic correctly handles end game events and updates the end game text accordingly.

src/Game/GameSystems.cpp (7)
  • 1-12: The inclusion of new header files "CreateMissiles.hpp" and "WaveSystemsAll.hpp" is noted. Ensure that these files exist and are located in the correct directory.

  • 34-40: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [17-38]

The function initPlayer has been modified to use the Types::Health struct instead of struct health_s. This change should be verified across the codebase to ensure that all calls to this function are updated accordingly.

  • 83-94: The new function sendMissileDeath is added. Ensure that this function is called appropriately in the codebase and that it behaves as expected.

  • 75-100: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [96-108]

The function sendEnemyDeath has been modified to remove a logging statement. This change should not affect the functionality of the function.

  • 123-146: The new function sendPlayerDeath is added. Ensure that this function is called appropriately in the codebase and that it behaves as expected.

  • 148-158: The GamePlugin::initPlugin function has been modified to add event callbacks for sendMissileDeath, sendPlayerDeath, and sendLifeUpdateToServer. Ensure that these callbacks are handled correctly in the event system.

  • 167-178: The bulletSystems and waveSystems vectors have been added to the gameSystems vector. This change should be verified to ensure that it does not introduce any unexpected behavior.

src/Server/Systems/WaveSystem.cpp (6)
  • 1-6: The file description in the header comment is now informative and provides a brief description of the purpose and functionality of the Wave Systems implementation.

  • 20-21: The addition of a static member variable _clockId and a mutex _mutex is good for thread safety. However, ensure that the mutex is used correctly throughout the code to avoid deadlocks.

  • 23-39: The constructor has been modified to initialize member variables and load wave data from a JSON file. Error handling is done using try-catch blocks which is good practice. However, consider separating the initialization of _wavesId into a separate function for better readability and maintainability.

  • 41-69: The startNextWave() function is well implemented with proper error handling and thread safety. However, consider separating the JSON data extraction into a separate function for better readability and maintainability.

  • 71-102: The functions isWaveEnded(), isGameEnded(), getMsBeforeNextWave(), isTimeBetweenWaves(), and setTimeBetweenWaves() are well implemented with proper thread safety. However, consider using std::atomic for _isGameEnded and _isTimeBetweenWaves if these variables are only being read and written, and not used in any complex operations. This could potentially improve performance by avoiding the overhead of locking and unlocking a mutex.

  • 106-130: The waveHandler() function is still complex. Consider breaking it down into smaller, more manageable functions. For example, you could create separate functions for handling the end of the game, the end of a wave, and the time between waves.

src/Nitwork/NitworkServer.hpp (7)
  • 54-74: The addStarWaveMessage and addConnectLobbyRespMsg functions have been added. Ensure that these functions are being called correctly throughout the codebase.

  • 109-115: The broadcastNewBulletMsg function has been renamed to broadcastNewMissileMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 140-151: The addMissileDeathMsg and addEndGameMsg functions have been added. Ensure that these functions are being called correctly throughout the codebase.

  • 195-229: The forkProcessAndCreateLobby, recreateLobby, and sendLobbyPid functions have been added. Ensure that these functions are being called correctly throughout the codebase.

  • 243-280: The handleConnectLobbyMsg and handleDisconnectMsg functions have been added. Ensure that these functions are being called correctly throughout the codebase.

  • 291-300: The _isGameStarted boolean variable has been added. Ensure that this variable is being used correctly throughout the codebase.

  • 306-383: The action handlers and action sender maps have been updated to include new message types. Ensure that these new message types are being handled correctly throughout the codebase.

src/Nitwork/NitworkMainServer.hpp (7)
  • 82-83: The addLobby function is added to the NitworkMainServer class. Ensure that it is used correctly in the codebase.

  • 85-89: The setIpOfMainServer function is added to the NitworkMainServer class. Ensure that it is used correctly in the codebase.

  • 91-95: The getAvailableIps function is added to the NitworkMainServer class. Ensure that it is used correctly in the codebase.

  • 119-124: The handleLobbyPidMsg function is added to the NitworkMainServer class. Ensure that it is used correctly in the codebase.

  • 158-161: The _ip member variable is added to the NitworkMainServer class. Ensure that it is used correctly in the codebase.

  • 205-232: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [164-218]

The _actionsHandlers map is updated to include the new action types and their corresponding handlers. Ensure that these new action types are handled correctly in the codebase.

  • 221-232: The _actionToSendHandlers map is updated to use the new action types NITWORK_LIST_LOBBY and NITWORK_CONNECT_MAIN_SERVER_RESP. Ensure that these new action types are handled correctly in the codebase.
src/Server/Systems/Network/ServerNetwork.cpp (3)
  • 19-28: The use of std::lock_guard is good for thread safety. However, consider using std::scoped_lock if you are using C++17 or later, as it is more flexible and can lock multiple mutexes at once.

  • 49-55: You are copying the arrHealth and arrPos arrays here. If these arrays are large, this could be a performance issue. Consider using references instead to avoid unnecessary copying.

- auto arrHealth   = registry.getComponents<Types::Health>();
- auto arrPos      = registry.getComponents<Types::Position>();
+ auto &arrHealth   = registry.getComponents<Types::Health>();
+ auto &arrPos      = registry.getComponents<Types::Position>();
  • 172-173: Similar to the previous comment, you are copying the arrHealth and arrPos arrays here. Consider using references instead to avoid unnecessary copying.
- auto arrHealth    = registry.getComponents<Types::Health>();
- auto arrPos       = registry.getComponents<Types::Position>();
+ auto &arrHealth    = registry.getComponents<Types::Health>();
+ auto &arrPos       = registry.getComponents<Types::Position>();
src/init.cpp (4)
  • 8-14: The inclusion of CreateLobbySystems.hpp is a new addition. Ensure that this file exists and is in the correct location.

  • 39-48: The addition of NETWORK_CREATE_LOBBY to the systems map is a new change. Ensure that the function getCreateLobbySystems exists in the CreateLobby namespace and returns the correct type.

  • 52-69: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [52-76]

The addition of createLobby to the sceneManager.setScenes() call is a new change. Ensure that the createLobby scene is correctly initialized and that it is intended to be in this order in the scenes list.

  • 80-81: As per the previous comment, the scene2 vector is declared and initialized but not used anywhere else. If it's not needed, consider removing it to improve code readability and maintainability.
-    auto scene2 = std::vector<std::size_t>();
-    sceneManager.setScenes({scene, scene2});
+    sceneManager.setScenes({scene});
src/Nitwork/NitworkMainServer.cpp (9)
  • 5-20: The inclusion of platform-specific headers and the order of inclusion is correct. This helps in avoiding potential conflicts between different platforms.

  • 62-67: The renaming of CONNECT_MAIN_SERVER_RESP to NITWORK_CONNECT_MAIN_SERVER_RESP is a good practice as it provides more context about the constant.

  • 75-83: The handleInitMsg function checks if a client is already connected before adding it to the _endpoints list. This is a good practice to avoid duplicate entries.

  • 139-156: The new function handleLobbyPidMsg handles lobby PID messages. Ensure that this function is being called appropriately in the codebase.

  • 171-177: The renaming of LIST_LOBBY to NITWORK_LIST_LOBBY is a good practice as it provides more context about the constant.

  • 190-217: The forkProcessAndCreateLobby function has been modified to pass the name as a command-line argument. This is a good practice as it allows for more flexibility and reusability of the function.

  • 253-257: The createLobby function now uses the _ip member variable and the local endpoint's port when calling forkProcessAndCreateLobby. Ensure that _ip is being set correctly elsewhere in the code.

  • 285-288: The new function setIpOfMainServer sets the IP of the main server. Ensure that this function is being called appropriately in the codebase.

  • 290-307: The new function getAvailableIps retrieves available IP addresses. This could be useful for network configuration or diagnostics. Ensure that this function is being called appropriately in the codebase.

src/Game/CreateMissiles.cpp (8)
  • 11-21: The inclusion of new headers is fine as long as they are necessary for the new functionality. Ensure that these headers are available in the project and that they don't introduce any circular dependencies.

  • 65-71: The playBulletSound function has been updated to use getMissileIdFromType instead of getMissileId. This is a good change if getMissileId was removed or if getMissileIdFromType provides additional functionality. Ensure that this change doesn't break any existing functionality that relied on getMissileId.

  • 82-96: The addSpriteRectsForBullet function has been updated to use Json::isDataExist instead of json.isDataExist. This is a good change if json.isDataExist was removed or if Json::isDataExist provides additional functionality. Ensure that this change doesn't break any existing functionality that relied on json.isDataExist.

  • 102-136: The createPlayerMissile function has been modified to return the created entity's ID. This is a good change if the ID is needed elsewhere in the code. However, ensure that this change doesn't break any existing functionality that relied on the previous return type (if any).

  • 139-170: The createEnemyMissile function has been modified to take Types::Velocity as a parameter. This is a good change if the velocity of the missile needs to be specified when creating an enemy missile. However, ensure that this change doesn't break any existing functionality that relied on the previous function signature.

  • 229-237: The launchMissileInLine, launchMissileInCircle, and launchEnemyMissile functions have been added. These functions seem to be well-implemented and provide useful functionality for launching missiles in different ways. However, ensure that these functions are used correctly throughout the codebase.

  • 239-258: The updateEnemiesAttacks function has been added. This function seems to be well-implemented and provides useful functionality for updating the attacks of enemies. However, ensure that this function is used correctly throughout the codebase.

  • 260-263: The getBulletsSystems function has been added. This function seems to be well-implemented and provides a way to get a vector of bullet system update functions. However, ensure that this function is used correctly throughout the codebase.

src/Server/Systems/WaveSystem.hpp (1)
  • 32-32: The static mutex _mutex is declared but not used anywhere in the class. If it's not needed, consider removing it. If it's for future use, add a comment to clarify this.
-        static std::mutex _mutex;

or

-        static std::mutex _mutex;
+        // The mutex is a placeholder for future use.
+        static std::mutex _mutex;
src/Client/Systems/Menus/Menu/Menu.cpp (10)
  • 24-49: The function initSpriteFromJson has been updated to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 51-71: The function initFromSprite has been updated to not return a value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 73-104: The function initInputBoxText has been added. This function initializes the text for an input box. It appears to be correctly implemented, but ensure that it is being called correctly throughout the codebase.

  • 21-117: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [106-127]

The function initInputBox has been updated to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 129-144: The function initButtonFromSprite has been added. This function initializes a button from a sprite and returns a std::size_t value. It appears to be correctly implemented, but ensure that it is being called correctly throughout the codebase.

  • 121-149: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [146-163]

The function initButtonFromRectangle has been added. This function initializes a button from a rectangle and returns a std::size_t value. It appears to be correctly implemented, but ensure that it is being called correctly throughout the codebase.

  • 166-193: The function initText has been added. This function initializes a text entity and returns a std::size_t value. It appears to be correctly implemented, but ensure that it is being called correctly throughout the codebase.

  • 195-202: The function initButton has been updated to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 204-220: The function initMenuEntity has been updated to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 222-234: The function initMenuSceneEntity has been added. This function initializes menu entities from an array of JSON data. It appears to be correctly implemented, but ensure that it is being called correctly throughout the codebase.

src/Client/Systems/Network/ClientNetwork.cpp (11)
  • 1-7: New headers have been included. Ensure that these headers are available and correctly referenced.

  • 19-34: The receiveLifeUpdate function has been updated to use the new Types::Health type instead of struct health_s. Ensure that the Types::Health type has the same structure and functionality as struct health_s.

  • 39-46: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [39-55]

The receiveEnemyDeath function has been updated to use the new Types::Health and Types::Enemy types. Ensure that these new types have the same structure and functionality as the previous types.

  • 58-78: A new function receiveMissileDeath has been added. This function handles the death of a missile by setting its health to 0 or removing it if it doesn't have a health component. Ensure that this function is called appropriately in the codebase.

  • 80-89: The handleStartWave function has been updated to set the wave ID and waiting status, and to add the initWave system. Ensure that these changes align with the intended game logic.

  • 175-182: A new function sendInitPacket has been added. This function sends an initialization packet and removes a system from a system manager. Ensure that this function is called appropriately in the codebase.

  • 262-274: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [252-272]

A new function receiveNewBullet has been added. This function handles the creation of a new bullet in the game. Ensure that this function is called appropriately in the codebase.

  • 375-389: A new function receiveConnectLobbyResp has been added. This function handles the response from a lobby connection attempt. Ensure that this function is called appropriately in the codebase.

  • 391-400: A new function quitGame has been added. This function handles the event of a player quitting the game. Ensure that this function is called appropriately in the codebase.

  • 402-408: A new function receiveEndGame has been added. This function handles the end of a game by adding the handleEndGameEvent system. Ensure that this function is called appropriately in the codebase.

  • 410-413: The getNetworkSystems function has been updated to include the new functions quitGame, sendInitPacket, sendPositionRelative, and sendPositionAbsolute. Ensure that these functions are correctly included in the returned vector.

src/Nitwork/NitworkClient.cpp (9)
  • 5-10: The preprocessor directive to disable warnings on Windows is a good practice for cross-platform compatibility. However, it's important to ensure that this doesn't hide any potential issues that could arise from using insecure functions.

  • 125-135: The connectMainServer function now returns a boolean indicating success or failure. This is a good practice as it allows the caller to handle the failure case appropriately. However, ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 144-160: The disconnectLobby function is correctly using a lock to ensure thread safety when accessing shared data. This is a good practice to prevent data races.

  • 166-172: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [162-172]

The addInitMsg function has been renamed to addConnectMainServerMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 331-350: The addMissileDeathMsg function has been added. This function appears to be correctly formed and follows the same pattern as other similar functions in this class.

  • 352-362: The addListLobbyMsg function has been renamed to addRequestListLobbyMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 377-383: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [377-387]

The addCreateLobbyMsg function has been renamed to addCreateLobbyRequestMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 407-413: The addConnectMainServerMsg function has been renamed to addConnectMainServerRequestMsg. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 421-436: The addConnectLobbyMsg function has been added. This function appears to be correctly formed and follows the same pattern as other similar functions in this class.

src/Game/GameCustomTypes.cpp Outdated Show resolved Hide resolved
Comment on lines +75 to 96
void receiveNewBulletMsg(const std::any &msg, boost::asio::ip::udp::endpoint & /* unused */)
{
std::lock_guard<std::mutex> lock(Registry::getInstance().mutex);
auto &arrMissiles = Registry::getInstance().getComponents<Types::Missiles>();
auto &arrHealth = Registry::getInstance().getComponents<Types::Health>();

const struct msgNewBullet_s &msgNewBullet = std::any_cast<struct msgNewBullet_s>(msg);
struct msgNewBullet_s msgNewBullet = std::any_cast<struct msgNewBullet_s>(msg);

struct Types::Position position = {
Maths::addIntDecimals(msgNewBullet.pos.x),
Maths::addIntDecimals(msgNewBullet.pos.y),
};
struct Types::Missiles missileType = {static_cast<missileTypes_e>(msgNewBullet.missileType)};
createMissile(position, missileType);
// send bullet to clients but not the sender
Nitwork::NitworkServer::getInstance().broadcastNewBulletMsg(msgNewBullet, endpoint);
struct Types::Missiles missileType = {msgNewBullet.missileType};
auto id = Systems::createPlayerMissile(position, missileType);
if (!arrMissiles.exist(id) || !arrHealth.exist(id)) {
Logger::error("Error: missile not created");
return;
}
msgNewBullet.id = arrMissiles[id].constId;
msgNewBullet.life = arrHealth[id].hp;
Nitwork::NitworkServer::getInstance().broadcastNewBulletMsg(msgNewBullet);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The endpoint parameter is unused in the receiveNewBulletMsg function. If it's not needed, consider removing it to avoid confusion. If it's intended to be used in the future, consider commenting on its purpose.

src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp Outdated Show resolved Hide resolved
src/Server/Systems/WaveSystem.hpp Show resolved Hide resolved
src/Server/Systems/WaveSystem.hpp Show resolved Hide resolved
src/Game/CreateMissiles.cpp Show resolved Hide resolved
Copy link
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.

Review Status

Actionable comments generated: 18

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 2b00212 and 30f7b28.
Files selected for processing (6)
  • src/Client/Systems/Events/EventsSystems.cpp (6 hunks)
  • src/Client/Systems/Menus/Menu/MenuSystems.cpp (1 hunks)
  • src/Game/GameCustomTypes.hpp (4 hunks)
  • src/Nitwork/NitworkMainServer.cpp (8 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
  • src/Nitwork/NitworkServer.hpp (6 hunks)
Files skipped from review due to trivial changes (1)
  • src/Nitwork/NitworkServer.cpp
Additional comments: 26
src/Nitwork/NitworkServer.hpp (6)
  • 54-74: The function addStarWaveMessage and addConnectLobbyRespMsg have been added. Ensure that these functions are being called correctly in the codebase and that the new parameter canConnect in addConnectLobbyRespMsg is being handled properly.

  • 109-115: The function broadcastNewBulletMsg has been renamed to broadcastNewMissileMsg and its parameter has been changed. Ensure that all calls to this function in the codebase have been updated to match the new function signature.

  • 140-151: The function addMissileDeathMsg and addEndGameMsg have been added. Ensure that these functions are being called correctly in the codebase.

  • 195-226: The functions forkProcessAndCreateLobby, recreateLobby, and sendLobbyPid have been added. Ensure that these functions are being called correctly in the codebase.

  • 240-277: The functions handleConnectLobbyMsg and handleDisconnectMsg have been added. Ensure that these functions are being called correctly in the codebase.

  • 288-373: The mappings of action types to corresponding handlers and senders have been updated. Ensure that these changes do not break any existing functionality and that all new action types are being handled correctly.

src/Game/GameCustomTypes.hpp (5)
  • 8-14: The inclusion of the new header file and the start of the namespace are fine.

  • 27-54: The Missiles struct has been updated to include a mutex and static variables. This is a good practice for thread safety when accessing shared data.

  • 60-70: The new EnemyAttack struct has been added. Ensure that all its member variables are used appropriately in the codebase.

  • 27-90: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [72-129]

The Enemy struct has been updated to include a mutex, static variables, and a new member variable attack of type EnemyAttack. This is a good practice for thread safety when accessing shared data. Also, ensure that the attack member variable is used appropriately in the codebase.

  • 131-228: The new WaveInfos class has been added. It includes several member variables and member functions. Ensure that all its member variables and functions are used appropriately in the codebase.
src/Client/Systems/Events/EventsSystems.cpp (7)
  • 18-24: The inclusion of "init.hpp" is new. Ensure that this header file exists and is in the correct location.

  • 120-126: The usage of struct health_s has been replaced with Types::Health. Ensure that this type exists and is correctly defined.

  • 137-141: The function Nitwork::NitworkClient::getInstance().addNewBulletMsg() is being called with new parameters. Ensure that the function signature has been updated to accept these parameters.

  • 164-174: The function playerMovement has been updated. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

  • 221-301: New functions isGameWin, modifEndGameText, and handleEndGameEvent have been added. Ensure that these functions are being called correctly in the codebase.

  • 237-269: The function modifEndGameText modifies the end game text based on the game outcome. Ensure that the text is being displayed correctly in the user interface.

  • 271-301: The function handleEndGameEvent handles end game events and updates the end game text accordingly. Ensure that this function is being called correctly in the codebase.

src/Nitwork/NitworkMainServer.cpp (7)
  • 5-20: The order of includes is correct, with system and third-party libraries first, followed by project-specific includes. This is a good practice to avoid hidden dependencies in the header files.

  • 75-83: The function handleInitMsg checks if the client is already connected before adding the endpoint to _endpoints. This is a good practice to prevent duplicate entries.

  • 139-157: The handleLobbyPidMsg function correctly handles the lobby PID message, removing any existing lobbies with the same name before adding the new one. This ensures that the lobby list is always up-to-date.

  • 172-178: The sendListLobby function correctly populates the msg.actionLobby array with the list of lobbies. However, consider checking if lobbies.size() exceeds the size of msg.actionLobby to prevent potential buffer overflow.

  • 254-258: The createLobby function correctly validates the input parameters before creating a new lobby. This is a good practice to ensure data integrity.

  • 282-284: The addLobby function correctly validates the input lobby before adding it to _lobbies. This is a good practice to ensure data integrity.

  • 286-289: The setIpOfMainServer function correctly sets the IP address of the main server. However, consider validating the input IP address before setting it.

src/Client/Systems/Menus/Menu/MenuSystems.cpp (1)
  • 229-252: The functions initHud() and `getGameTextures

src/Nitwork/NitworkMainServer.cpp Outdated Show resolved Hide resolved
src/Client/Systems/Menus/Menu/MenuSystems.cpp Outdated Show resolved Hide resolved
src/Client/Systems/Menus/Menu/MenuSystems.cpp Outdated Show resolved Hide resolved
Copy link
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.

Review Status

Actionable comments generated: 4

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 30f7b28 and 2a1cb3b.
Files ignored due to filter (1)
  • assets/Json/playerData.json
Files selected for processing (4)
  • src/Client/Systems/Events/EventsSystems.cpp (6 hunks)
  • src/Game/GameCustomTypes.cpp (1 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
  • src/Nitwork/NitworkServer.hpp (7 hunks)
Additional comments: 23
src/Nitwork/NitworkServer.hpp (5)
  • 57-57: The function getServerInfos() is declared but not defined in this file. Ensure that it is defined in the corresponding .cpp file.

  • 76-76: The function addConnectLobbyRespMsg() is a new addition. Ensure that it is used correctly in the codebase and that it is defined in the corresponding .cpp file.

  • 252-252: The function handleConnectLobbyMsg() is a new addition. Ensure that it is used correctly in the codebase and that it is defined in the corresponding .cpp file.

  • 306-384: The _actionsHandlers map has been updated with new action types and handlers. Ensure that these new action types are handled correctly in the codebase and that the handlers are defined in the corresponding .cpp file.

  • 388-449: The _actionToSendHandlers map has been updated with new action types and handlers. Ensure that these new action types are handled correctly in the codebase and that the handlers are defined in the corresponding .cpp file.

src/Game/GameCustomTypes.cpp (3)
  • 16-17: The static member variable _missileNb and the mutex _mutex are added to the Types::Missiles class. Ensure that these are used correctly in the rest of the codebase to avoid data races.

  • 21-26: The prepareNextWave function is added to the Types::WaveInfos class. This function sets the first enemy as not created, clears the remaining enemies, and sets waiting for the next wave. Ensure that this function is called at the appropriate times in the game logic.

  • 28-67: The setWaitingForNextWave function is updated to add/remove entities and update text when CLIENT is defined. Ensure that this function is called at the appropriate times in the game logic and that the entities are correctly added/removed.

src/Client/Systems/Events/EventsSystems.cpp (1)
  • 93-102: The previous comments about adding more comments for clarity and avoiding repeated calculations are still valid. The calculations for adjusting missile position are complex and could benefit from additional comments for clarity. Also, the conversion between integer and float types is done multiple times. Consider storing the results of these conversions in variables to avoid repeated calculations.
src/Nitwork/NitworkServer.cpp (14)
  • 8-23: The code includes several headers and checks for the Windows platform. Ensure that all the necessary headers are included and that the platform-specific code is correct.

  • 89-94: The code uses reinterpret_cast to cast the data in _receiveBuffer to a struct action_s *. This is a potentially dangerous operation and should be used with caution. Ensure that the data in _receiveBuffer is indeed of type struct action_s.

  • 151-154: The code is accessing multiple components from the Registry instance. It would be more maintainable and readable if these component accesses were abstracted into separate functions.

  • 186-197: The code checks if the game has already started or if the maximum number of players has been reached. If either of these conditions is true, it sets canConnect to false. This logic could be simplified by returning early when these conditions are met, which would make the code easier to read and understand.

  • 204-208: The code checks if the game has already started or if the maximum number of players has been reached. If either of these conditions is true, it returns early. This is a good practice as it simplifies the logic and makes the code easier to read.

  • 253-271: The code checks if the game has already started or if the client is not connected. If either of these conditions is true, it returns early. This is a good practice as it simplifies the logic and makes the code easier to read. The code also sets _isGameStarted to true and adds the waveHandler system when all players are ready.

  • 310-334: The code creates a new process on Windows using CreateProcess. Ensure that the command line arguments are correct and that the process is created successfully.

  • 335-361: The code creates a new process on Unix-like systems using fork and execl. Ensure that the command line arguments are correct and that the process is created successfully.

  • 402-420: The code handles a disconnect message. It removes the disconnected client from _endpoints and _playersReady, and sends a player death message. If all clients have disconnected, it recreates the lobby and stops the scene manager. Ensure that all necessary cleanup is performed when a client disconnects.

  • 426-437: The code creates a packetConnectLobbyResp_s packet and adds it to the send queue. Ensure that the packet is created correctly and that it is sent successfully.

  • 544-555: The code creates a packetNewBullet_s packet and sends it to all clients. Ensure that the packet is created correctly and that it is sent successfully.

  • 623-641: The code creates a packetMissileDeath_s packet and adds it to the send queue. Ensure that the packet is created correctly and that it is sent successfully.

  • 643-653: The code creates a packetEndGame_s packet and sends it to all clients. Ensure that the packet is created correctly and that it is sent successfully.

  • 654-655: The code retrieves the player ID for a given endpoint. It uses at instead of [] to access the map element, which is a good practice as it throws an exception if the element does not exist.

src/Client/Systems/Events/EventsSystems.cpp Show resolved Hide resolved
@Saverio976 Saverio976 mentioned this pull request Nov 5, 2023
12 tasks
Copy link
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.

Review Status

Actionable comments generated: 10

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 2a1cb3b and 74b73e8.
Files selected for processing (3)
  • src/Client/Systems/Events/EventsSystems.cpp (6 hunks)
  • src/Game/GameCustomTypes.cpp (1 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
Additional comments: 10
src/Game/GameCustomTypes.cpp (4)
  • 16-17: A new global variable Types::Missiles::_missileNb and a corresponding mutex _mutex are added. Ensure that these are used correctly and safely across threads to avoid data races.

  • 21-26: The prepareNextWave function is added. Ensure that it is called at the appropriate places in the codebase.

  • 28-67: The setWaitingForNextWave function is added and it manipulates entities and components in the Registry class. Ensure that these manipulations are safe and do not lead to any undefined behavior or memory leaks.

  • 30-65: The #ifdef CLIENT directive is used to conditionally compile code. Ensure that the code within this directive is tested in both client and non-client builds.

src/Client/Systems/Events/EventsSystems.cpp (1)
  • 221-234: The function isGameWin checks if both idsPlayer and idsOtherPlayer are empty to determine if the game is won. This logic seems incorrect as it implies that the game is won when there are no players. Please verify this logic.
src/Nitwork/NitworkServer.cpp (5)
  • 8-12: The code includes Windows-specific headers and defines a macro to suppress warnings about unsafe C Runtime functions. This is a common practice when writing cross-platform code. However, it's generally better to use safer alternatives to these functions where possible.

  • 151-154: The code is accessing multiple components from the Registry instance. It would be more maintainable and readable if these component accesses were abstracted into separate functions.

  • 186-197: This code checks if the game has already started or if the maximum number of players has been reached. If either of these conditions is true, it sets canConnect to false. This logic could be simplified by returning early when these conditions are met, which would make the code easier to read and understand.

  • 310-334: The code uses the Windows API function CreateProcess to create a new process. This is a common way to create a new process on Windows, but it's important to ensure that the command line arguments are properly sanitized to prevent command injection attacks.

  • 335-361: The code uses the Unix fork and execl functions to create a new process. This is a common way to create a new process on Unix-like systems, but it's important to ensure that the command line arguments are properly sanitized to prevent command injection attacks.

src/Game/GameCustomTypes.cpp Show resolved Hide resolved
src/Game/GameCustomTypes.cpp Show resolved Hide resolved
src/Client/Systems/Events/EventsSystems.cpp Show resolved Hide resolved
src/Client/Systems/Events/EventsSystems.cpp Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Show resolved Hide resolved
src/Nitwork/NitworkServer.cpp Show resolved Hide resolved
KitetsuK and others added 2 commits November 5, 2023 22:01
* CLIENT-MENU: Add the possibility for client to create servers
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-MENU: Add a little text to indicate scene + fix quiGame bug
MINOR

* CLIENT-MENU: Fix bad merging conflicts
PATCH

* CLIENT-MENU: Now client can create only 1 server
MINOR

* CLIENT-MENU: Fix windows compilation
PATCH

* FORMAT-AUTO: automatic format on pull request #142

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-MENU: Now users can choose parallax
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-GAME: Fix compile

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-MENU: Now users can choose parallax
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-GAME: Add music menu / Remove debug

* CLIENT-MENU: Now users can choose parallax
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT: Remove log unused

PATCH

* CLIENT-MENU: Now users can know if they already create a server
MINOR

* CLIENT-MENU: Now users can click on a button to exit programm
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-MENU: Modif text button
MINOR

* CLIENT-MENU: Modif now text button can reappeared even if the scene is changed
MINOR

* FORMAT-AUTO: automatic format on pull request #142

* CLIENT-MENU: Modif now it s working
MINOR

---------

Co-authored-by: Github Actions <github-actions[bot]@users.noreply.github.com>
Co-authored-by: tenshi <[email protected]>
Co-authored-by: Xavier Mitault <[email protected]>
@Saverio976 Saverio976 merged commit ef3829e into dev Nov 5, 2023
12 checks passed
github-actions bot added a commit that referenced this pull request Nov 5, 2023
Copy link
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.

Review Status

Actionable comments generated: 39

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 74b73e8 and 6d8bd07.
Files ignored due to filter (5)
  • assets/Audio/Musics/home.mp3
  • assets/Json/createLobby.json
  • assets/Json/createServer.json
  • assets/Json/menu.json
  • assets/Json/selectLobby.json
Files selected for processing (18)
  • src/Client/Systems/Menus/CMakeLists.txt (1 hunks)
  • src/Client/Systems/Menus/CreateServer/CMakeLists.txt (1 hunks)
  • src/Client/Systems/Menus/CreateServer/CreateServerSystems.cpp (1 hunks)
  • src/Client/Systems/Menus/CreateServer/CreateServerSystems.hpp (1 hunks)
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.cpp (1 hunks)
  • src/Client/Systems/Menus/Menu/ButtonCallbacks.hpp (1 hunks)
  • src/Client/Systems/Menus/Menu/Menu.cpp (3 hunks)
  • src/Client/Systems/Menus/Menu/MenuSystems.cpp (1 hunks)
  • src/Client/Systems/Network/ClientNetwork.cpp (12 hunks)
  • src/Client/Systems/Parallax/Parallax.cpp (2 hunks)
  • src/Client/Systems/Parallax/Parallax.hpp (1 hunks)
  • src/Game/ResourcesManager.hpp (1 hunks)
  • src/Nitwork/NitworkClient.cpp (14 hunks)
  • src/Nitwork/NitworkClient.hpp (19 hunks)
  • src/Nitwork/NitworkMainServer.cpp (8 hunks)
  • src/Nitwork/NitworkServer.cpp (20 hunks)
  • src/init.cpp (4 hunks)
  • src/init.hpp (1 hunks)
Files skipped from review due to trivial changes (3)
  • src/Client/Systems/Menus/CMakeLists.txt
  • src/Client/Systems/Menus/CreateServer/CreateServerSystems.cpp
  • src/init.hpp
Additional comments: 60
src/Client/Systems/Menus/CreateServer/CMakeLists.txt (3)
  • 1-1: The minimum required version of CMake is set to 3.15. Ensure that this version is compatible with the rest of the project and the build environment.

  • 3-7: The target_include_directories command is used to specify the private include directories for the ${PROJECT_NAME_CLIENT}. This is a good practice as it helps in encapsulating the dependencies.

  • 9-13: The target_sources command is used to specify the private source file CreateServerSystems.cpp for the ${PROJECT_NAME_CLIENT}. This is a good practice as it helps in managing the source files for each target.

src/Game/ResourcesManager.hpp (2)
  • 10-22: The JsonType enum has been updated. Ensure that all references to the old enum values have been updated to the new ones throughout the codebase. Also, verify that the new enum values are being used correctly.

  • 26-38: The ResourcesManagerValues::paths vector has been updated. Ensure that all references to the old paths have been updated to the new ones throughout the codebase. Also, verify that the new paths are correct and the files they point to exist.

The rest of the code in the ResourcesManager class seems to be unchanged and does not require any comments.

src/init.cpp (4)
  • 8-15: The new include statements "CreateLobbySystems.hpp" and "CreateServerSystems.hpp" are added. Ensure these files exist and are in the correct path.

  • 40-50: Two new system managers "NETWORK_CREATE_LOBBY" and "NETWORK_CREATE_SERVER" are added. Ensure that the corresponding functions "getCreateLobbySystems" and "getCreateServerSystems" exist and return the correct type.

  • 54-78: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [54-84]

The initialization of scenes is updated to include the new system managers. Ensure that the order of system managers in the scene initialization matches the intended execution order.

  • 89-90: The scene2 vector is declared and initialized but not used anywhere else. If it's not needed, consider removing it to improve code readability and maintainability.
-    auto scene2 = std::vector<std::size_t>();
-    sceneManager.setScenes({scene, scene2});
+    sceneManager.setScenes({scene});

This comment is still valid from the previous review.

src/Client/Systems/Parallax/Parallax.cpp (3)
  • 1-5: The includes are well ordered and the necessary files are included. Good job.

  • 6-8: The namespace and static member variable are correctly defined.

  • 44-51: The use of std::lock_guard is a good practice for thread safety. The data retrieval from JSON is also done correctly. However, consider handling exceptions that might be thrown by Json::getInstance().getDataByJsonType().

src/Nitwork/NitworkClient.cpp (3)
  • 126-138: The connectMainServer function now returns a boolean indicating success or failure. This is a good practice as it allows the caller to handle the situation when the connection fails. However, ensure that all calls to this function throughout the codebase have been updated to handle the returned boolean.

  • 432-481: The createForkedServer function creates a forked server process. It's good to see platform-specific code for Windows and Unix-like systems. However, ensure that the server creation process is thoroughly tested on both types of systems.

  • 483-499: The stop function stops the server processes. It's good to see platform-specific code for Windows and Unix-like systems. However, ensure that the server stopping process is thoroughly tested on both types of systems.

src/Client/Systems/Parallax/Parallax.hpp (1)
  • 1-37: The singleton pattern is used here, which is fine. However, the static member _actualParallaxNbr is declared but not used anywhere in the class. If it's not used, consider removing it to avoid confusion.
- static std::size_t _actualParallaxNbr;

Also, the setActualParralaxType() function has a typo in its name. It should be setActualParallaxType().

- void setActualParralaxType(JsonType data)
+ void setActualParallaxType(JsonType data)
src/Nitwork/NitworkClient.hpp (10)
  • 37-40: The function connectMainServer now returns a boolean value. Ensure that all calls to this function throughout the codebase have been updated to handle the returned value.

  • 46-62: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [42-52]

The function disconnectLobby has been added. Ensure that it is being used correctly in the codebase.

  • 54-59: The function addInitMsg has been moved from private to public. Ensure that this change does not violate encapsulation principles and that the function is not being misused in the codebase.

  • 103-104: The function addMissileDeathMsg has been added. Ensure that it is being used correctly in the codebase.

  • 123-134: The functions serverAlreadyCreated, createForkedServer, and stop have been added. Ensure that they are being used correctly in the codebase.

  • 158-163: The function addConnectLobbyMsg has been added. Ensure that it is being used correctly in the codebase.

  • 234-236: The variable _serverPids has been added. Ensure that it is being used correctly in the codebase.

  • 370-375: The NITWORK_MISSILE_DEATH action has been added to the _actionsHandlers map. Ensure that it is being used correctly in the codebase.

  • 403-408: The NITWORK_END_GAME action has been added to the _actionsHandlers map. Ensure that it is being used correctly in the codebase.

  • 490-493: The NITWORK_DISCONNECT_LOBBY action has been added to the _actionToSendHandlers map. Ensure that it is being used correctly in the codebase.

src/Client/Systems/Menus/Menu/Menu.cpp (10)
  • 24-49: The function initSpriteFromJson has been updated to return a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 51-71: The function initFromSprite has been split from initSpriteFromJson. This is a good practice as it improves the modularity and readability of the code.

  • 73-104: The function initInputBoxText has been added. This function initializes the text of an input box. It's a good practice to separate this logic into its own function for better readability and maintainability.

  • 21-117: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [106-127]

The function initInputBox has been updated to include a call to initInputBoxText. This is a good practice as it improves the modularity of the code.

  • 129-146: The function initButtonFromSprite now returns a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 162-237: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [148-166]

The function initButtonFromRectangle now returns a std::size_t value. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.

  • 168-195: The function initText has been added. This function initializes text entities. It's a good practice to separate this logic into its own function for better readability and maintainability.

  • 197-204: The function initButton has been updated to call either initButtonFromSprite or initButtonFromRectangle based on the existence of a "spritePath" in the JSON element. This is a good practice as it improves the modularity of the code.

  • 207-222: The function initMenuEntity has been updated to handle different object types and return a std::size_t value. This is a good practice as it improves the modularity and flexibility of the code.

  • 224-236: The function initMenuSceneEntity has been added. This function initializes menu entities from a JSON array. It's a good practice to separate this logic into its own function for better readability and maintainability.

src/Client/Systems/Network/ClientNetwork.cpp (11)
  • 1-7: New includes have been added. Ensure that these files exist and are accessible from the current file's location.

  • 19-34: The receiveLifeUpdate function has been updated to use the Types::Health component instead of struct health_s. This change should be verified across the codebase to ensure consistency.

  • 39-46: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [39-56]

The receiveEnemyDeath function has been updated to use the Types::Health component instead of struct health_s. This change should be verified across the codebase to ensure consistency.

  • 58-78: A new function receiveMissileDeath has been added. This function handles the death of a missile by setting its health to 0 or removing it if it doesn't have a health component. The function seems to be thread-safe as it uses a lock guard.

  • 80-89: The handleStartWave function has been modified to set the wave ID and update the wave status. Ensure that these changes are consistent with the rest of the codebase.

  • 98-103: The receiveNewEnemy function has been updated to use the Types::Health component instead of struct health_s. This change should be verified across the codebase to ensure consistency.

  • 109-117: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [105-118]

The createNewPlayer function has been updated to use the Types::Health component instead of struct health_s. This change should be verified across the codebase to ensure consistency.

  • 175-182: A new function sendInitPacket has been added. This function sends an initialization packet and removes a system from a system manager. Ensure that the removal of the system doesn't cause any issues in the rest of the codebase.

  • 262-274: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [252-272]

The receiveNewBullet function has been modified to set the missile ID and health. Ensure that these changes are consistent with the rest of the codebase.

  • 375-389: A new function receiveConnectLobbyResp has been added. This function handles the response from a lobby connection attempt. It changes the scene to the game scene if the connection is successful, otherwise, it logs that the lobby is full.

  • 391-397: A new function receiveEndGame has been added. This function handles end game events by adding the handleEndGameEvent system to the EVENTS system manager. Ensure that the handleEndGameEvent system is designed to handle end game events correctly.

src/Nitwork/NitworkMainServer.cpp (7)
  • 5-20: The includes are correctly ordered and the platform-specific includes are properly guarded.

  • 75-83: The handleInitMsg function correctly checks if the client is already connected before adding the endpoint to the _endpoints vector. However, consider adding error handling or logging if the client is already connected.

  • 139-157: The handleLobbyPidMsg function correctly handles lobby PID messages. However, consider adding error handling or logging if the lobby name is not found in the _lobbies vector.

  • 191-217: The forkProcessAndCreateLobby function correctly forks a process and creates a lobby. However, consider handling potential errors when creating the process on Windows and when forking on other platforms.

  • 253-257: The createLobby function correctly validates the inputs before creating a lobby. However, consider adding error handling or logging if the lobby creation fails.

  • 281-283: The addLobby function correctly validates the inputs before adding a lobby to the _lobbies vector. However, consider adding error handling or logging if the lobby addition fails.

  • 285-288: The setIpOfMainServer function correctly sets the IP address of the main server. However, consider validating the input IP address before setting it.

src/Nitwork/NitworkServer.cpp (6)
  • 8-12: Ensure that the Windows-specific includes and macros are necessary and used in the code. If not, consider removing them to reduce complexity.

  • 310-334: This block of code is creating a new process in Windows. Ensure that the command line arguments are correct and that error handling is in place for the CreateProcess function.

  • 335-361: This block of code is creating a new process in Unix-like systems. Ensure that the command line arguments are correct and that error handling is in place for the fork and execl functions.

  • 388-398: Ensure that the sendLobbyPid function is correctly sending the process ID to the client.

  • 402-420: Ensure that the handleDisconnectMsg function is correctly handling client disconnections and that the lobby is correctly recreated when all clients have disconnected.

  • 645-653: Ensure that the new message creation functions are correctly creating and sending the appropriate messages.

Comment on lines +1 to +16
/*
** EPITECH PROJECT, 2023
** R-Bus
** File description:
** CreateServerSystems
*/

#pragma once

#include <functional>
#include <vector>

namespace Systems::CreateServerSystems {
std::vector<std::function<void(std::size_t /*unused*/, std::size_t /*unused*/)>>
getCreateServerSystems();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The new file and namespace are well-structured and follow good practices. However, the function getCreateServerSystems() takes two parameters that are marked as unused. If these parameters are not needed, consider removing them to avoid confusion. If they are placeholders for future use, consider documenting this in a comment.

-    std::vector<std::function<void(std::size_t /*unused*/, std::size_t /*unused*/)>>
-    getCreateServerSystems();
+    std::vector<std::function<void()>>
+    getCreateServerSystems();

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
/*
** EPITECH PROJECT, 2023
** R-Bus
** File description:
** CreateServerSystems
*/
#pragma once
#include <functional>
#include <vector>
namespace Systems::CreateServerSystems {
std::vector<std::function<void(std::size_t /*unused*/, std::size_t /*unused*/)>>
getCreateServerSystems();
}
/*
** EPITECH PROJECT, 2023
** R-Bus
** File description:
** CreateServerSystems
*/
#pragma once
#include <functional>
#include <vector>
namespace Systems::CreateServerSystems {
std::vector<std::function<void()>> getCreateServerSystems();
}

Comment on lines 10 to 12
static void initParallaxEntity(nlohmann::basic_json<> &elem, const int maxOffsideParallax = 0)
{
std::size_t id = Registry::getInstance().addEntity();
Copy link
Contributor

Choose a reason for hiding this comment

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

The function initParallaxEntity is well defined. However, the default value for maxOffsideParallax is not documented. Consider adding a comment to explain its purpose and why it defaults to 0.

Comment on lines 141 to +145
void NitworkClient::connectLobby(const std::string &ip, n_port_t port)
{
setLobbyEndpoint(ip, port);
addInitMsg();
addConnectLobbyMsg();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The connectLobby function doesn't handle any exceptions or errors. It's recommended to add error handling to this function similar to connectMainServer.

void NitworkClient::connectLobby(const std::string &ip, n_port_t port)
{
+   try {
        setLobbyEndpoint(ip, port);
        addConnectLobbyMsg();
+   } catch (const std::exception &e) {
+       Logger::error("Failed to connect to lobby: " + std::string(e.what()));
+   }
}

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
void NitworkClient::connectLobby(const std::string &ip, n_port_t port)
{
setLobbyEndpoint(ip, port);
addInitMsg();
addConnectLobbyMsg();
}
void NitworkClient::connectLobby(const std::string &ip, n_port_t port)
{
try {
setLobbyEndpoint(ip, port);
addConnectLobbyMsg();
} catch (const std::exception &e) {
Logger::error("Failed to connect to lobby: " + std::string(e.what()));
}
}

Comment on lines +19 to +22
void setActualParralaxType(JsonType data)
{
_actualParallax = data;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The setActualParallaxType() function does not perform any validation on the input data. Depending on the range of valid values for JsonType, you might want to add some error checking here to ensure that only valid values are set.

Comment on lines +14 to +17
JsonType getActualParallaxType()
{
return _actualParallax;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The getActualParallaxType() function directly exposes the internal state of the class. This might be fine depending on your use case, but consider returning a const reference instead to prevent outside code from modifying the internal state.

- JsonType getActualParallaxType()
+ const JsonType& getActualParallaxType() const

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
JsonType getActualParallaxType()
{
return _actualParallax;
}
const JsonType& getActualParallaxType() const
{
return _actualParallax;
}

Comment on lines +104 to +111
static void deleteInputBoxChar(std::size_t id, Registry::components<Types::InputBox> &arrInputBox)
{
auto &arrText = Registry::getInstance().getComponents<Raylib::TextShared>();

if (arrInputBox[id].text.size() > 0) {
arrInputBox[id].text.pop_back();
arrText[id]->setCurrentText(arrInputBox[id].text);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function deleteInputBoxChar() has been updated but still removes the last character from the text property of a specific InputBox if the text property is not empty. This function could be optimized by checking if the text property is empty before attempting to remove the last character. This would prevent unnecessary string manipulation and improve performance.

Comment on lines +114 to +133
void checkInputDeletion(std::size_t, std::size_t)
{
constexpr std::size_t delay = 150;
Registry::components<Types::InputBox> arrInputBox =
Registry::getInstance().getComponents<Types::InputBox>();
std::vector<std::size_t> ids = Registry::getInstance().getEntitiesByComponents(
{typeid(Types::InputBox), typeid(Raylib::TextShared)});
static auto clockId = Registry::getInstance().getClock().create();

if (Raylib::KeyboardInput::isKeyDown(Raylib::KeyboardKey::KB_BACKSPACE)
&& Registry::getInstance().getClock().elapsedMillisecondsSince(clockId) > delay) {
for (auto id : ids) {
if (arrInputBox[id].selected) {
Registry::getInstance().getClock().restart(clockId);
deleteInputBoxChar(id, arrInputBox);
return;
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function checkInputDeletion() has been updated but still iterates over all entities with an InputBox and TextShared component and calls deleteInputBoxChar() if the selected property of the InputBox is true and the backspace key is down. This function could be optimized by only iterating over entities that have selected set to true. This would reduce the number of iterations and improve performance.

Comment on lines +135 to +155
void hoverInputBox(std::size_t /*unused*/, std::size_t /*unused*/)
{
Registry::components<Types::AnimRect> arrAnimRect =
Registry::getInstance().getComponents<Types::AnimRect>();
Registry::components<Types::InputBox> arrInputBox =
Registry::getInstance().getComponents<Types::InputBox>();
std::vector<std::size_t> ids = Registry::getInstance().getEntitiesByComponents(
{typeid(Types::CollisionRect), typeid(Types::Position), typeid(Types::InputBox)});

for (auto id : ids) {
if (::Menu::checkIsInsideRect(id)) {
if (arrAnimRect.exist(id) && !arrInputBox[id].selected) {
arrAnimRect[id].changeRectList(Types::RectListType::HOVER);
}
return;
}
if (arrAnimRect.exist(id) && !arrInputBox[id].selected) {
arrAnimRect[id].changeRectList(Types::RectListType::DEFAULT_RECT);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function hoverInputBox() has been updated but still iterates over all entities with a CollisionRect, Position, and InputBox component and changes the RectListType of the AnimRect to HOVER if the mouse is inside the CollisionRect and the selected property of the InputBox is false. This function could be optimized by only iterating over entities that have selected set to false. This would reduce the number of iterations and improve performance.

Comment on lines +157 to +169
static void manageClick(std::size_t id, Registry::components<Types::AnimRect> &arrAnimRect)
{
Registry::components<Types::Button> arrButton =
Registry::getInstance().getComponents<Types::Button>();

arrAnimRect[id].changeRectList(Types::RectListType::HOVER);
if (Raylib::MouseInput::isMouseButtonDown(Raylib::MouseButton::MOUSE_BTN_LEFT)) {
arrAnimRect[id].changeRectList(Types::RectListType::SELECTED);
}
if (Raylib::MouseInput::isMouseButtonReleased(Raylib::MouseButton::MOUSE_BTN_LEFT)) {
arrButton[id].callback();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function manageClick() has been updated but still changes the RectListType of a specific AnimRect to HOVER and calls the callback() function of the Button if the left mouse button is released. This function could be optimized by only changing the RectListType of the AnimRect to HOVER if it was not already HOVER. This would prevent unnecessary function calls and improve performance.

Comment on lines +171 to +189
void pressButton(std::size_t /*unused*/, std::size_t /*unused*/)
{
Registry::components<Types::AnimRect> arrAnimRect =
Registry::getInstance().getComponents<Types::AnimRect>();

std::vector<std::size_t> ids = Registry::getInstance().getEntitiesByComponents(
{typeid(Types::CollisionRect),
typeid(Types::AnimRect),
typeid(Types::Position),
typeid(Types::Button)});

for (auto id : ids) {
if (::Menu::checkIsInsideRect(id)) {
manageClick(id, arrAnimRect);
return;
}
arrAnimRect[id].changeRectList(Types::RectListType::UNDEFINED);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The function pressButton() has been updated but still iterates over all entities with a CollisionRect, AnimRect, Position, and Button component and calls manageClick() if the mouse is inside the CollisionRect. This function could be optimized by only iterating over entities that have RectListType set to UNDEFINED. This would reduce the number of iterations and improve performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants