-
Notifications
You must be signed in to change notification settings - Fork 0
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
The merge #143
Conversation
Merge MINOR
…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>
WalkthroughA 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 Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this 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
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
tosetText
. Ensure that all references to this function in the codebase have been updated to reflect this change. Also, verify that the new functionsetText
is being used correctly with the new parameter typeconst std::string &text
.src/Client/Systems/Menus/Menu/Menu.hpp (3)
15-19: The new enum value
SPRITE
has been added toObjectType
. Ensure that all code that uses this enum is updated to handle this new value.61-64: The function
initMenuEntity
now returns astd::size_t
value and takes anlohmann::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
, andreceiveEndGame
have been added to theSystems
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
andpageMax
. 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 constructorLobbyStatus
now has default parameter values forip
andport
. Make sure that all instances ofLobbyStatus
are created with these default values in mind.src/Client/Systems/Graphic/GraphicSystems.cpp (3)
21-44: The
debugCollisionRect
function is only compiled whenNDEBUG
is not defined. This is a common practice for debug functions, but make sure thatNDEBUG
is defined in production builds to avoid unnecessary overhead.24-24: The
std::lock_guard
is used to ensure thread safety when accessing theRegistry
singleton. This is a good practice, but make sure that all other accesses to theRegistry
are also protected by a mutex to avoid data races.46-61: The
getGraphicsSystems
function has been modified to include thedebugCollisionRect
function in theaudioSystems
vector. This is fine as long as thedebugCollisionRect
function signature matches the expected function signature for the vector. Also, ensure that thedebugCollisionRect
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 theRegistry
singleton. The change fromJsonType::PARALLAX
toJsonType::DEFAULT_PARALLAX
is consistent with the PR summary. Ensure that theJsonType::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 theSystems
namespace returns a vector ofstd::function
objects. Ensure that the function is implemented correctly and that the returnedstd::function
objects are used properly throughout the codebase. Also, make sure that thestd::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
andmodifEndGameText
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 theisGameWin
andmodifEndGameText
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
andpageMax
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 includeinitLobbyRow
andupdateLobbyRow
. 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 specifiedendpoint
. 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 theTypes::Missiles
class. Ensure that these variables are used correctly and thread-safely throughout the codebase.20-25: The
prepareNextWave()
function is added to theTypes::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 theTypes::WaveInfos
class. This function contains code that interacts with theRegistry
andRaylib
classes whenCLIENT
is defined. Ensure that this function is used correctly and that theCLIENT
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 insetMissileNb
andgetMissileNb
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 insetEnemyNb
andgetEnemyNb
functions is a good practice. The addition of theEnemyAttack
member variableattack
and thegetAttack
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 ingetInstance
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
toMAGICK_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 thestd::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 beingstruct packetListLobby_s
. This is a good use ofif constexpr
andstd::is_same_v
to perform compile-time checks. However, ensure that the log levelLogger::fatal
is appropriate for this situation.122-122: The new function
deletePacketFromEndPoints
is added to theNitworkServer
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 astd::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 newinitInputBoxText
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 astd::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 astd::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 eitherinitButtonFromSprite
orinitButtonFromRectangle
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 singlenlohmann::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
andWaveSystemsAll.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 theTypes::Health
struct instead ofstruct 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 thestruct health_s
struct instead ofTypes::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
andgetAvailableIps
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_RESPsrc/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 whenids
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
andTypes::Enemy
components are correctly initialized and updated throughout the codebase. Also, consider adding error handling for the case whenids
is empty.
58-90: The
handleStartWave
function has been updated to useTypes::WaveInfos
instead ofTypes::Enemy
. Ensure that this change is reflected throughout the codebase and thatTypes::WaveInfos
is correctly initialized and updated.99-104: The
receiveNewEnemy
function has been updated to useTypes::Health
instead ofstruct health_s
. Ensure that this change is reflected throughout the codebase and thatTypes::Health
is correctly initialized and updated.110-118: The
createNewPlayer
function has been updated to useTypes::Health
instead ofstruct health_s
. Ensure that this change is reflected throughout the codebase and thatTypes::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 useTypes::Missiles
andTypes::Health
instead ofstruct health_s
. Ensure that this change is reflected throughout the codebase and thatTypes::Missiles
andTypes::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
andgetMissileIdFromType
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 usegetMissileIdFromType
instead ofgetMissileId
. 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 useJson::isDataExist
instead ofjson.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:
ThecreateLobby
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 fromvoid
tobool
. 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 theSceneManager
before settingisRunning
to false. This is a good practice as it ensures that theSceneManager
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 withInputBox
andAnimRect
components and sets theselected
field ofInputBox
tofalse
and changes theRectListType
ofAnimRect
toDEFAULT_RECT
. This function seems to be used to deselect all input boxes.45-62: The
setInputBoxSelected
function sets theselected
field of theInputBox
component of the entity with the givenid
totrue
and changes theRectListType
of itsAnimRect
component toSELECTED
. This function seems to be used to select a specific input box.77-89: The
checkTextInput
function checks if anyInputBox
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 theRectListType
of itsAnimRect
component toHOVER
. 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 theRectListType
of itsAnimRect
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 themanageClick
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 theinitMenu
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 theinitSceneGame
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 toNITWORK_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 toNITWORK_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 ther-type_server
process is a good practice. It allows ther-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
andgetAvailableIps
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
, andaddEndGameMsg
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
andaddEndGameMsg
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 toboost::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 theSystems::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 theenumListTosizet
function. Also, verify that thecreateLobby
vector is used correctly in the subsequent code.74-74: Ensure that the
sceneManager.setScenes()
function can handle the newcreateLobby
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 abool
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 returnsfalse
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 apacketDisconnectLobby_s
structure and adding it to the send queue. The use of astd::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 toaddConnectLobbyMsg
. 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 apacketMissileDeath_s
structure and adding it to the send queue. The use of astd::lock_guard
to protect the_receivedPacketsIdsMutex
is a good practice for thread safety.352-362: The
addListLobbyMsg
function has been renamed toaddRequestListLobbyMsg
. 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 toaddCreateLobbyRequestMsg
. 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 toaddConnectMainServerRequestMsg
. 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 apacketConnectLobby_s
structure and adding it to the send queue. The use of astd::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
, andDISCONNECT_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
andEND_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 theCONNECT_MAIN_SERVER
action has been updated to19
. Ensure that this change is reflected in the client and server code where this action is used.230-230: The
magick
value for theCREATE_LOBBY
action has been updated to23
. 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
, andDISCONNECT_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
andEND_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.
There was a problem hiding this 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
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 forenemy_type_e
to provide stronger type safety and better encapsulation.46-64: The
getEnemyId
andgetEnemyType
functions are still well implemented with error handling. However, consider usingstd::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
andTERMINATORBOSS
) 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
, andENEMIES
) are being used correctly.24-34: The
ResourcesManagerValues::paths
vector has been updated to reflect the changes in theJsonType
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 ofEnemyAttack
. This is a good practice for thread safety when accessing shared data. ThegetAttack
function has been added to access theattack
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
toMAGICK_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 awaveId
field instead ofenemyNb
. Make sure to update all references to this struct throughout the codebase.128-134: The
msgNewBullet_s
struct now includes anid
field and alife
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 amsg
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 theTypes::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 theCLIENT
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 theSELECT_LOBBY
scene and initialize the parallax effect and menu entities. Ensure that theSELECT_LOBBY
scene and theParallax::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 theRaylib::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 withTypes::Health
. Ensure that this type exists and is correctly defined.221-269: The new functions
isGameWin
andmodifEndGameText
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 theTypes::Health
struct instead ofstruct 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 forsendMissileDeath
,sendPlayerDeath
, andsendLifeUpdateToServer
. Ensure that these callbacks are handled correctly in the event system.167-178: The
bulletSystems
andwaveSystems
vectors have been added to thegameSystems
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()
, andsetTimeBetweenWaves()
are well implemented with proper thread safety. However, consider usingstd::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
andaddConnectLobbyRespMsg
functions have been added. Ensure that these functions are being called correctly throughout the codebase.109-115: The
broadcastNewBulletMsg
function has been renamed tobroadcastNewMissileMsg
. Ensure that all calls to this function throughout the codebase have been updated to match the new name.140-151: The
addMissileDeathMsg
andaddEndGameMsg
functions have been added. Ensure that these functions are being called correctly throughout the codebase.195-229: The
forkProcessAndCreateLobby
,recreateLobby
, andsendLobbyPid
functions have been added. Ensure that these functions are being called correctly throughout the codebase.243-280: The
handleConnectLobbyMsg
andhandleDisconnectMsg
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 theNitworkMainServer
class. Ensure that it is used correctly in the codebase.85-89: The
setIpOfMainServer
function is added to theNitworkMainServer
class. Ensure that it is used correctly in the codebase.91-95: The
getAvailableIps
function is added to theNitworkMainServer
class. Ensure that it is used correctly in the codebase.119-124: The
handleLobbyPidMsg
function is added to theNitworkMainServer
class. Ensure that it is used correctly in the codebase.158-161: The
_ip
member variable is added to theNitworkMainServer
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 typesNITWORK_LIST_LOBBY
andNITWORK_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 usingstd::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
andarrPos
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
andarrPos
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 thesystems
map is a new change. Ensure that the functiongetCreateLobbySystems
exists in theCreateLobby
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 thesceneManager.setScenes()
call is a new change. Ensure that thecreateLobby
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
toNITWORK_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
toNITWORK_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 callingforkProcessAndCreateLobby
. 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 usegetMissileIdFromType
instead ofgetMissileId
. This is a good change ifgetMissileId
was removed or ifgetMissileIdFromType
provides additional functionality. Ensure that this change doesn't break any existing functionality that relied ongetMissileId
.82-96: The
addSpriteRectsForBullet
function has been updated to useJson::isDataExist
instead ofjson.isDataExist
. This is a good change ifjson.isDataExist
was removed or ifJson::isDataExist
provides additional functionality. Ensure that this change doesn't break any existing functionality that relied onjson.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 takeTypes::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
, andlaunchEnemyMissile
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 astd::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 astd::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 astd::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 astd::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 astd::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 astd::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 astd::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 newTypes::Health
type instead ofstruct health_s
. Ensure that theTypes::Health
type has the same structure and functionality asstruct 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 newTypes::Health
andTypes::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 theinitWave
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 thehandleEndGameEvent
system. Ensure that this function is called appropriately in the codebase.410-413: The
getNetworkSystems
function has been updated to include the new functionsquitGame
,sendInitPacket
,sendPositionRelative
, andsendPositionAbsolute
. 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 toaddConnectMainServerMsg
. 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 toaddRequestListLobbyMsg
. 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 toaddCreateLobbyRequestMsg
. 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 toaddConnectMainServerRequestMsg
. 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.
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); | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this 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
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
andaddConnectLobbyRespMsg
have been added. Ensure that these functions are being called correctly in the codebase and that the new parametercanConnect
inaddConnectLobbyRespMsg
is being handled properly.109-115: The function
broadcastNewBulletMsg
has been renamed tobroadcastNewMissileMsg
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
andaddEndGameMsg
have been added. Ensure that these functions are being called correctly in the codebase.195-226: The functions
forkProcessAndCreateLobby
,recreateLobby
, andsendLobbyPid
have been added. Ensure that these functions are being called correctly in the codebase.240-277: The functions
handleConnectLobbyMsg
andhandleDisconnectMsg
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 variableattack
of typeEnemyAttack
. This is a good practice for thread safety when accessing shared data. Also, ensure that theattack
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 withTypes::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
, andhandleEndGameEvent
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 themsg.actionLobby
array with the list of lobbies. However, consider checking iflobbies.size()
exceeds the size ofmsg.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
There was a problem hiding this 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
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 theTypes::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 theTypes::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 whenCLIENT
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 astruct action_s *
. This is a potentially dangerous operation and should be used with caution. Ensure that the data in_receiveBuffer
is indeed of typestruct 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 thewaveHandler
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
andexecl
. 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.
There was a problem hiding this 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
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 theRegistry
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 bothidsPlayer
andidsOtherPlayer
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
andexecl
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.
* 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]>
There was a problem hiding this 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
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 fileCreateServerSystems.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 byJson::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 besetActualParallaxType()
.- 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
, andstop
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 astd::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 frominitSpriteFromJson
. 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 toinitInputBoxText
. This is a good practice as it improves the modularity of the code.
129-146: The function
initButtonFromSprite
now returns astd::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 astd::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 eitherinitButtonFromSprite
orinitButtonFromRectangle
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 astd::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 theTypes::Health
component instead ofstruct 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 theTypes::Health
component instead ofstruct 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 theTypes::Health
component instead ofstruct 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 theTypes::Health
component instead ofstruct 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 thehandleEndGameEvent
system to theEVENTS
system manager. Ensure that thehandleEndGameEvent
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
andexecl
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.
/* | ||
** 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(); | ||
} |
There was a problem hiding this comment.
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.
/* | |
** 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(); | |
} |
static void initParallaxEntity(nlohmann::basic_json<> &elem, const int maxOffsideParallax = 0) | ||
{ | ||
std::size_t id = Registry::getInstance().addEntity(); |
There was a problem hiding this comment.
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.
void NitworkClient::connectLobby(const std::string &ip, n_port_t port) | ||
{ | ||
setLobbyEndpoint(ip, port); | ||
addInitMsg(); | ||
addConnectLobbyMsg(); | ||
} |
There was a problem hiding this comment.
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.
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())); | |
} | |
} |
void setActualParralaxType(JsonType data) | ||
{ | ||
_actualParallax = data; | ||
} |
There was a problem hiding this comment.
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.
JsonType getActualParallaxType() | ||
{ | ||
return _actualParallax; | ||
} |
There was a problem hiding this comment.
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.
JsonType getActualParallaxType() | |
{ | |
return _actualParallax; | |
} | |
const JsonType& getActualParallaxType() const | |
{ | |
return _actualParallax; | |
} |
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); | ||
} |
There was a problem hiding this comment.
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.
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; | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.
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); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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.
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(); | ||
} | ||
} |
There was a problem hiding this comment.
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.
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); | ||
} | ||
} |
There was a problem hiding this comment.
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.
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