Feature/rb 136 other player distroyed when player die #124
Conversation
by merging them MINOR
on server side PATCH
…of github.com:X-R-G-B/R-Bus into feature/RB-136-other-player-distroyed-when-player-die
…layer-distroyed-when-player-die
systems.cpp PATCH
|
Warning Rate Limit Exceeded@romainpanno has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 0 minutes and 40 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, convert this PR to a draft and then mark it as ready for review again to re-trigger the review. Alternatively, you can push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per repository. WalkthroughThis pull request introduces significant changes to the game's network protocol, player and enemy death handling, and player creation. It enhances the game's functionality and communication between the client and server by adding new fields to data structures, renaming actions, and adding new actions. It also improves code maintainability and readability. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (15)
- docs/network/rfc/RFC.md (9 hunks)
- src/Client/Systems/Graphic/DeathSystems.cpp (2 hunks)
- src/Client/Systems/Network/ClientNetwork.cpp (5 hunks)
- src/Client/Systems/Network/ClientNetwork.hpp (1 hunks)
- src/ECS/Systems/Systems.cpp (6 hunks)
- src/ECS/Systems/Systems.hpp (1 hunks)
- src/Nitwork/ANitwork.cpp (3 hunks)
- src/Nitwork/ANitwork.hpp (2 hunks)
- src/Nitwork/Nitwork.h (7 hunks)
- src/Nitwork/NitworkClient.cpp (2 hunks)
- src/Nitwork/NitworkClient.hpp (5 hunks)
- src/Nitwork/NitworkServer.cpp (5 hunks)
- src/Nitwork/NitworkServer.hpp (7 hunks)
- src/Server/Systems/Network/ServerNetwork.cpp (1 hunks)
- src/Server/Systems/Network/ServerNetwork.hpp (1 hunks)
Additional comments (Suppressed): 51
src/Server/Systems/Network/ServerNetwork.hpp (1)
- 18-18: The new function
receivePlayerDeathMsghas been added to handle player death messages. Ensure that this function is implemented correctly and that it is called in the appropriate places in the codebase. Also, verify that the function properly handles themsgandendpointparameters.src/Client/Systems/Network/ClientNetwork.hpp (2)
12-12: The function
receiveNewPlayerhas been added. Ensure that this function is implemented correctly and that it handles the new player data as expected. Also, verify that the function is being called in the appropriate places in the codebase.17-17: The function
receivePlayerDeathhas been added. Ensure that this function is implemented correctly and that it handles the player death data as expected. Also, verify that the function is being called in the appropriate places in the codebase.src/Client/Systems/Graphic/DeathSystems.cpp (2)
16-16: The
setPlayerAnimRectDeathfunction has been changed from a lambda function to a regular function. Ensure that this change does not affect any other parts of the codebase where this function is used.29-29: The
deathFunctionsmap has been updated to include a death function for theTypes::OtherPlayertype. This is a good addition as it extends the death handling functionality to other players in the game.src/Nitwork/ANitwork.hpp (2)
99-99: The change from
sizeof(struct header_s)toHEADER_SIZEseems to be a refactoring change. Ensure thatHEADER_SIZEis correctly defined and equivalent tosizeof(struct header_s). IfHEADER_SIZEis not equivalent, it could lead to incorrect memory access, causing undefined behavior.169-174: The
_packetIdmember variable has been removed. Ensure that this does not affect any functionality where_packetIdwas used. If_packetIdwas used in any logic, removing it could cause issues.src/Nitwork/NitworkClient.cpp (2)
52-52: The change from
sizeof(struct header_s)toHEADER_SIZEis a good practice as it makes the code more readable and maintainable. However, please ensure thatHEADER_SIZEis correctly defined and equals the size ofheader_sstructure to avoid any potential issues.227-248: The new method
addPlayerDeathMsgis introduced to handle the creation and sending of player death messages. It uses a mutex lock to ensure thread safety when accessing_receivedPacketsIdsMutex. This is a good practice for preventing data races in multithreaded environments. However, please ensure that the lock is released in all possible execution paths to avoid potential deadlocks.src/Nitwork/Nitwork.h (4)
34-35: The magic numbers for
MAGICK_NEW_PLAYERandMAGICK_PLAYER_DEATHhave been added. Ensure that these magic numbers are unique and not used elsewhere in the codebase. Also, verify that these magic numbers are handled correctly in the network protocol.52-55: The action types
NEW_PLAYERandPLAYER_DEATHhave been added to then_actionType_tenumeration. Ensure that these new action types are handled correctly in the network protocol and that theN_ACTION_TYPE_MAXvalue is updated accordingly.181-187: The
msgCreatePlayer_sstructure has been modified to include new fieldspos,life, andisOtherPlayer. Ensure that these fields are initialized correctly when creating a new player and that they are used correctly in the network protocol.222-225: A new structure
msgPlayerDeath_shas been added to handle player death messages. Ensure that this structure is used correctly in the network protocol and that theplayerIdfield is set correctly when a player dies.src/Nitwork/ANitwork.cpp (3)
- 13-13: The initialization of
_packetIdhas been removed from the constructor. If_packetIdis used elsewhere in the code, ensure that it is properly initialized before use to avoid undefined behavior.- ANitwork::ANitwork() : _socket(_context), _packetId(0) + ANitwork::ANitwork() : _socket(_context)
- 140-140: The check for packet size has been changed from
sizeof(struct header_s)toHEADER_SIZE. Ensure thatHEADER_SIZEis correctly defined and is equivalent tosizeof(struct header_s). If not, this could lead to incorrect behavior when receiving packets.- if (packetSize > MAX_PACKET_SIZE || packetSize < sizeof(struct header_s)) { + if (packetSize > MAX_PACKET_SIZE || packetSize < HEADER_SIZE) {
- 239-239: The error message for an action not found in
actionToSendHandlershas been improved to include the action that was not found. This is a good change as it will make debugging easier by providing more context in the error message.- Logger::error("NITWORK: action not found"); + Logger::error("NITWORK: action not found: " + std::to_string(data.action));src/Client/Systems/Network/ClientNetwork.cpp (4)
1-1: The new hunk includes the
CustomTypes.hppheader file. Ensure that this file exists and contains the necessary definitions for the custom types used in the code.37-45: The code now checks if the health component exists for the enemy before setting its health to 0. This is a good practice as it prevents potential runtime errors if the health component does not exist for a particular enemy.
76-97: The
createNewPlayerfunction has been significantly modified. It now checks if a player or other player with the same ID already exists, and if so, removes that entity before initializing a new one. This could potentially prevent duplication of player entities. However, ensure that removing an existing player entity does not have unintended side effects elsewhere in the code.243-265: The new function
receivePlayerDeathhandles the death of a player. It sets the health of the player or other player with the matching ID to 0. This is a good way to represent the death of a player in the game. However, ensure that other parts of the code properly handle a player's health being set to 0, such as removing the player from the game or triggering a respawn.src/Nitwork/NitworkClient.hpp (5)
39-39: The new method
addPlayerDeathMsgis introduced to handle player death messages. Ensure that this method is called appropriately in the codebase whenever a player death event occurs.72-75: The
handleBodyfunction now usesmsgCreatePlayer_sinstead ofmsgPlayerInit_sand thereceiveNewPlayerfunction is called instead ofreceivePlayerInit. Verify that these changes are consistent with the new network protocol and that themsgCreatePlayer_sstruct andreceiveNewPlayerfunction are implemented correctly.124-130: The
NEW_ALLIEaction has been replaced withNEW_PLAYER. ThehandleBodyfunction now usesmsgCreatePlayer_sinstead ofmsgNewAllie_sand thereceiveNewPlayerfunction is called instead ofreceiveNewAllie. Ensure that these changes are consistent with the new network protocol and that themsgCreatePlayer_sstruct andreceiveNewPlayerfunction are implemented correctly.166-176: A new
PLAYER_DEATHaction is introduced. ThehandleBodyfunction usesmsgPlayerDeath_sand thereceivePlayerDeathfunction is called. Ensure that these are implemented correctly and that thePLAYER_DEATHaction is handled appropriately in the codebase.224-229: The
sendDatafunction now usespacketPlayerDeath_sfor thePLAYER_DEATHaction. Verify thatpacketPlayerDeath_sis implemented correctly and that thePLAYER_DEATHaction is handled appropriately when sending data.src/Nitwork/NitworkServer.hpp (5)
40-40: The function signature for
addPlayerInitMessagehas been changed to accept amsgCreatePlayer_sobject instead of a player ID. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that themsgCreatePlayer_sobject contains all the necessary information that was previously provided by the player ID.50-54: New functions
addPlayerDeathMsgandaddNewPlayerMsghave been added. Ensure that these functions are being called at the appropriate places in the codebase to handle player death and the addition of new players. Also, verify that the necessary data is being passed to these functions.78-82: The function
sendNewAlliehas been modified to accept apacketCreatePlayer_sobject instead of apacketNewAllie_sobject. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that thepacketCreatePlayer_sobject contains all the necessary information that was previously provided by thepacketNewAllie_sobject.150-156: A new action
PLAYER_DEATHhas been added to the_actionsHandlersmap. Ensure that this action is being handled correctly in theSystems::receivePlayerDeathMsgfunction and that the necessary data is being passed to this function.196-197: The
sendDatafunction is now being called with different packet types (packetCreatePlayer_sandpacketPlayerDeath_s) for theINIT,NEW_PLAYER, andPLAYER_DEATHactions. Ensure that thesendDatafunction can handle these new packet types and that all necessary data is being included in these packets.src/ECS/Systems/Systems.cpp (6)
341-341: The
deadListvariable is now a reference to the list of dead components. This change is good for performance as it avoids unnecessary copying of the list. However, ensure that the list is not modified elsewhere in a way that could invalidate this reference.361-373: The code now checks if the dead entity is a player or other player and sends a death message accordingly. This is a new feature introduced in this PR. Ensure that the
addPlayerDeathMsgfunction handles these messages correctly and that the server and client code can process these messages.386-402: The
sendDeathMsgfunction has been modified to handle death messages for both players and enemies. This is a significant change from the previous version which only handled enemy death messages. Ensure that the server and client code can process these new types of messages.418-418: The
sendDeathMsgfunction is now called when an entity's health reaches zero. This is a change from the previous version wheresendEnemyDeathwas called. This change is part of the new feature introduced in this PR where death messages are sent for both players and enemies.466-474: The
initPlayerfunction now takes additional parameters for position and health. This is a significant change from the previous version which only took a constant ID and a boolean indicating if the player is another player. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.501-501: The
positionvariable is now initialized with the passed in position instead of a default position. This is a significant change from the previous version where the position was initialized with a default value. Ensure that the passed in position is valid and that this change does not affect the gameplay or player spawning mechanics.docs/network/rfc/RFC.md (8)
43-45: The addition of "Position", "Life", and "Is Other Player" fields to the server action in the network protocol is noted. Ensure that these fields are properly handled in the server-side code and that the client-side code is updated to correctly interpret these new fields.
104-110: The renaming of the "NEW_ALLIE" action to "NEW_PLAYER" and the addition of "Position", "Life", and "Is Other Player" fields are noted. Make sure that all instances of "NEW_ALLIE" in the codebase are updated to "NEW_PLAYER" and that these new fields are correctly handled.
125-131: The addition of the "PLAYER_DEATH" action is noted. Ensure that this action is correctly handled in both the client-side and server-side code, and that the "Player ID" field is correctly interpreted.
260-263: The renaming of the "NEW_ALLIE" action to "NEW_PLAYER" and the addition of the "PLAYER_DEATH" action in the action body are noted. Make sure that all instances of "NEW_ALLIE" in the codebase are updated to "NEW_PLAYER" and that the "PLAYER_DEATH" action is correctly handled.
290-292: The addition of "pos", "life", and "isOtherPlayer" fields to the server action is noted. Ensure that these fields are properly handled in the server-side code and that the client-side code is updated to correctly interpret these new fields.
310-342: The detailed specifications for the new "Position", "Life", and "Is Other Player" fields are noted. Ensure that these specifications are correctly implemented in the server-side code and that the client-side code is updated to correctly interpret these new fields.
720-782: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [714-778]
The renaming of the "NEW_ALLIE" action to "NEW_PLAYER" and the detailed specifications for the new "Position", "Life", and "Is Other Player" fields are noted. Make sure that all instances of "NEW_ALLIE" in the codebase are updated to "NEW_PLAYER" and that these new fields are correctly handled.
- 868-915: The addition of the "PLAYER_DEATH" action and the detailed specifications for the "Player ID" field are noted. Ensure that this action and field are correctly handled in both the client-side and server-side code.
src/Nitwork/NitworkServer.cpp (7)
- 70-70: The change from
sizeof(struct header_s)toHEADER_SIZEis not inherently problematic, but it's important to ensure thatHEADER_SIZEis defined correctly and matches the size ofheader_s. IfHEADER_SIZEis not equal tosizeof(struct header_s), it could lead to incorrect memory access and undefined behavior.// Verify that HEADER_SIZE is defined correctly static_assert(HEADER_SIZE == sizeof(struct header_s), "HEADER_SIZE does not match the size of header_s");
113-130: The function
sendNewAlliehas been modified to take apacketCreatePlayer_sinstead ofpacketNewAllie_s. Ensure that all calls to this function have been updated to match the new signature. Also, the function now sets theplayerIdfield of thepacketMsgCreatePlayerargument, which could potentially overwrite existing data. If this is not the intended behavior, consider passingplayerIdas a separate argument.133-165: The new function
sendAlliesAlreadyPresentsends information about existing players to a newly connected player. This is a good addition for maintaining game state consistency across all players. However, the function uses a lot of nested loops and conditionals, which increases its complexity. Consider refactoring this function to improve readability and maintainability.181-196: The
handleInitMsgfunction has been significantly modified. It now uses apacketCreatePlayer_sstructure instead ofpacketNewAllie_s, and it initializes the player using theSystems::initPlayerfunction with additional parameters for position and health. Ensure that these changes are compatible with the rest of the codebase. Also, the function now sends apacketCreatePlayer_spacket to all clients, which could potentially overwrite existing data. If this is not the intended behavior, consider passingplayerIdas a separate argument.247-260: The
addPlayerInitMessagefunction has been modified to take amsgCreatePlayer_sinstead of an_id_t. Ensure that all calls to this function have been updated to match the new signature. Also, the function now sets themsgfield of thepacketCreatePlayerargument, which could potentially overwrite existing data. If this is not the intended behavior, consider passingplayerMsgas a separate argument.361-373: The new function
addPlayerDeathMsgsends apacketPlayerDeath_spacket to all clients when a player dies. This is a good addition for maintaining game state consistency across all players. However, the function uses astd::lock_guardto lock_receivedPacketsIdsMutex, which could potentially lead to deadlocks if not handled properly. Ensure that this mutex is always unlocked, even in case of exceptions.375-390: The new function
addNewPlayerMsgsends apacketCreatePlayer_spacket to a specific client. This is a good addition for maintaining game state consistency across all players. However, the function uses astd::lock_guardto lock_receivedPacketsIdsMutex, which could potentially lead to deadlocks if not handled properly. Ensure that this mutex is always unlocked, even in case of exceptions.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (9)
- src/Client/Systems/Graphic/DeathSystems.cpp (2 hunks)
- src/Client/Systems/Network/ClientNetwork.cpp (5 hunks)
- src/ECS/Systems/Systems.cpp (6 hunks)
- src/ECS/Systems/Systems.hpp (1 hunks)
- src/Nitwork/ANitwork.hpp (2 hunks)
- src/Nitwork/NitworkClient.cpp (2 hunks)
- src/Nitwork/NitworkServer.cpp (5 hunks)
- src/Nitwork/NitworkServer.hpp (7 hunks)
- src/Server/Systems/Network/ServerNetwork.cpp (1 hunks)
Additional comments (Suppressed): 30
src/ECS/Systems/Systems.hpp (1)
- 32-36: The function signature of
initPlayerhas been changed to include two new parameters:const struct position_absolute_s &posandconst struct health_s &life. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. Also, verify that the new parameters are being used correctly within the function.src/Nitwork/ANitwork.hpp (2)
98-99: The reinterpret cast has been modified to use
HEADER_SIZEinstead ofsizeof(struct header_s). Ensure thatHEADER_SIZEis equivalent tosizeof(struct header_s). If not, this could lead to incorrect memory access.169-174: The member variable
_packetIdhas been removed. Ensure that this does not affect any functionality where_packetIdwas used.src/Client/Systems/Graphic/DeathSystems.cpp (2)
16-17: The
setPlayerAnimRectDeathfunction has been changed from a lambda function to a regular function. Ensure that this change does not affect any other parts of the codebase where this function might be used.29-30: The
deathFunctionsmap has been updated to include a new entity typeTypes::OtherPlayer. This change seems to be in line with the PR's goal of handling death notifications for other players. However, it's important to ensure that thesetPlayerAnimRectDeathfunction can handleTypes::OtherPlayerentities correctly.src/Server/Systems/Network/ServerNetwork.cpp (1)
- 116-151: The
receivePlayerDeathMsgfunction is responsible for handling player death messages. It locks the registry mutex to ensure thread safety, retrieves the necessary components from the registry, and then iterates over the entities that have these components. If it finds an entity with the same ID as the player who died, it checks if the player's health is greater than 0. If it is, it sends a new player message to the endpoint. If the player is not found, it logs a debug message.This function seems to be well-structured and follows good practices for thread safety and error handling. However, there is a potential issue with the logic in lines 133-146. If the player's health is greater than 0, it sends a new player message. This seems counterintuitive as the function is supposed to handle player death messages. Please verify if this is the intended behavior.
Also, the ternary operator in lines 141-144 could be simplified. The comparison itself returns a boolean value, so the ternary operator is unnecessary.
- .isOtherPlayer = (Nitwork::NitworkServer::getInstance().getPlayerId(endpoint) - != otherPlayer.constId) - ? true - : false, + .isOtherPlayer = Nitwork::NitworkServer::getInstance().getPlayerId(endpoint) != otherPlayer.constId,src/Nitwork/NitworkClient.cpp (2)
52-52: The reinterpret cast is now using
HEADER_SIZEinstead ofsizeof(struct header_s). Ensure thatHEADER_SIZEis correctly defined and equivalent tosizeof(struct header_s). If not, this could lead to incorrect memory access.227-244: The new function
addPlayerDeathMsgis added to send a player death message to the server. It uses a mutex lock to ensure thread safety when accessing_receivedPacketsIdsMutex. This is a good practice for preventing data races in a multithreaded environment. However, ensure that the lock is released in all possible execution paths to avoid deadlocks.The function constructs a
packetPlayerDeath_sstructure and sends it as a packet. Ensure that theheaderandactionfields are correctly initialized. Currently, theheaderfield is initialized with all zeros, which might not be correct depending on the protocol specification. Also, theactionfield only has themagickfield set toPLAYER_DEATH, ensure that this is the intended behavior.The function uses
std::make_anyto create astd::anyobject frompacketPlayerDeath. This is a good practice for creating type-safe containers. However, ensure that the receiver of this packet correctly handlesstd::anyand knows how to extractpacketPlayerDeath_sfrom it.src/Client/Systems/Network/ClientNetwork.cpp (4)
3-3: The inclusion of "CustomTypes.hpp" is new. Ensure that this header file exists and is necessary for the code changes in this PR.
37-45: The logic for handling enemy death has been modified. Previously, the enemy entity was removed from the registry. Now, the enemy's health is set to 0 if it exists. This change could have implications on how the game handles enemy entities after their death. Ensure that this change is intended and that it doesn't introduce bugs or unexpected behavior in the game.
76-101: The
createNewPlayerfunction is a new addition. It creates a new player or other player entity in the game. The function first checks if a player or other player with the same ID already exists. If so, it removes the existing entity before creating a new one. This could potentially cause issues if the existing player is still in use or referenced elsewhere in the code. Ensure that removing existing players is safe and doesn't cause any issues.247-271: The
receivePlayerDeathfunction is a new addition. It handles the death of a player or other player entity. The function sets the health of the dying player to 0. This is a significant change to the game's death handling logic and could have implications on how the game handles player deaths. Ensure that this change is intended and that it doesn't introduce bugs or unexpected behavior in the game.src/Nitwork/NitworkServer.hpp (6)
40-42: The function signature for
addPlayerInitMessagehas been changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. The new signature now includes amsgCreatePlayer_sparameter.52-56: New functions
addPlayerDeathMsgandaddNewPlayerMsghave been added. These functions seem to handle player death and new player messages respectively. Ensure that these functions are used correctly in the codebase.80-84: The function signature for
sendNewAlliehas been changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature. The new signature now includes apacketCreatePlayer_sparameter instead ofpacketNewAllie_s.152-158: A new action
PLAYER_DEATHhas been added to the_actionsHandlersmap. This action seems to handle player death messages. Ensure that this action is used correctly in the codebase.185-187: The
NEW_ALLIEaction has been renamed toNEW_PLAYERin the_actionToSendHandlersmap. Ensure that this change is reflected throughout the codebase.197-200: A new action
PLAYER_DEATHhas been added to the_actionToSendHandlersmap. This action seems to handle player death messages. Ensure that this action is used correctly in the codebase.src/Nitwork/NitworkServer.cpp (6)
70-70: The reinterpret cast is now using
HEADER_SIZEinstead ofsizeof(struct header_s). Ensure thatHEADER_SIZEis correctly defined and equivalent tosizeof(struct header_s). If not, this could lead to incorrect memory access.113-130: The function
sendNewAllienow takes apacketCreatePlayer_sinstead ofpacketNewAllie_s. Ensure that all calls to this function have been updated to match the new signature. Also, thepacketCreatePlayer_sstructure seems to have more fields thanpacketNewAllie_s. Make sure these additional fields are correctly handled in the function.179-204: The
handleInitMsgfunction now initializes a player with position and health parameters. Ensure that these parameters are correctly set in thepacketMsgCreatePlayerstructure. Also, the function now callssendAlliesAlreadyPresentafter sending the new ally. Make sure this does not introduce any race conditions or other issues.255-270: The
addPlayerInitMessagefunction now takes amsgCreatePlayer_sinstead of a player ID. Ensure that all calls to this function have been updated to match the new signature. Also, themsgCreatePlayer_sstructure seems to have more fields than just a player ID. Make sure these additional fields are correctly handled in the function.371-383: The new function
addPlayerDeathMsgsends aPLAYER_DEATHmessage to all clients. Ensure that this message is correctly handled on the client side and that the player death is correctly reflected in the game state.385-400: The new function
addNewPlayerMsgsends aNEW_PLAYERmessage to a specific client. Ensure that this message is correctly handled on the client side and that the new player is correctly added to the game state.src/ECS/Systems/Systems.cpp (6)
341-344: The
deadListvariable is now a reference instead of a copy. This change is good for performance as it avoids unnecessary copying. However, ensure that the original data is not modified unintentionally through this reference.361-373: The
executeDeathFunctionnow checks if the dead entity is a player or other player and sends a death message accordingly. This is a new feature introduced in this PR. Ensure that theaddPlayerDeathMsgfunction handles these messages correctly.386-402: The
sendEnemyDeathfunction is renamed tosendDeathMsgand now handles death messages for both enemies and other players. This is a good change for code maintainability as it reduces code duplication. However, ensure that theaddPlayerDeathMsgandaddEnemyDeathMsgfunctions handle these messages correctly.418-420: The
deathCheckerfunction now callssendDeathMsginstead ofsendEnemyDeath. This change is consistent with the renaming of thesendEnemyDeathfunction tosendDeathMsg.466-478: The
initPlayerfunction now takes additional parameters for position and health. This change provides more flexibility in player initialization. However, ensure that all calls to this function throughout the codebase have been updated to match the new signature.505-507: The
initPlayerfunction now directly uses the passed position for player initialization. This change is consistent with the new parameter added to the function. However, ensure that the passed position is valid and within the game window boundaries.
health_s is not present PATCH
…of github.com:X-R-G-B/R-Bus into feature/RB-136-other-player-distroyed-when-player-die
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- docs/network/rfc/RFC.md (9 hunks)
- src/Client/Systems/Network/ClientNetwork.cpp (5 hunks)
Additional comments (Suppressed): 11
src/Client/Systems/Network/ClientNetwork.cpp (3)
3-3: The inclusion of the "CustomTypes.hpp" header file is new. Ensure that this file exists and is in the correct location. Also, verify that the types defined in this file are used in the new code.
37-47: The new code introduces a health component for enemies. This is a significant change from the previous logic where enemies were simply removed upon death. Now, the health of an enemy is set to 0 upon death. If an enemy does not have a health component, a fatal error is logged and the enemy is removed. This change could have implications for game mechanics and should be thoroughly tested.
249-273: The
receivePlayerDeathfunction is a new addition. It handles player death messages by setting the health of the player or other player to 0. This is a significant change from the previous logic where player death was not handled. This change could have implications for game mechanics and should be thoroughly tested.docs/network/rfc/RFC.md (8)
43-45: The addition of "Position", "Life", and "Is Other Player" fields to the server's data structure is noted. Ensure that these fields are properly initialized and used in the server-side code. Also, verify that the client-side code has been updated to handle these new fields.
104-110: The renaming of the "NEW_ALLIE" action to "NEW_PLAYER" and the addition of "PLAYER_DEATH" action is noted. Make sure that all instances of "NEW_ALLIE" in the codebase have been updated to "NEW_PLAYER". Also, ensure that the "PLAYER_DEATH" action is properly handled in both the client-side and server-side code.
125-131: The addition of the "PLAYER_DEATH" action is noted. Ensure that this action is properly handled in both the client-side and server-side code. Also, verify that the "Player ID" field is correctly used to identify the player who died.
260-263: The addition of "NEW_PLAYER" and "PLAYER_DEATH" actions to the list of actions is noted. Make sure that these actions are properly handled in the codebase.
290-292: The addition of "pos", "life", and "isOtherPlayer" fields to the server action is noted. Ensure that these fields are properly initialized and used in the server-side code. Also, verify that the client-side code has been updated to handle these new fields.
310-344: The addition of "Position", "Life", and "Is Other Player" fields to the server's data structure is noted. Ensure that these fields are properly initialized and used in the server-side code. Also, verify that the client-side code has been updated to handle these new fields.
722-786: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [716-782]
The renaming of the "NEW_ALLIE" action to "NEW_PLAYER" and the addition of "PLAYER_DEATH" action is noted. Make sure that all instances of "NEW_ALLIE" in the codebase have been updated to "NEW_PLAYER". Also, ensure that the "PLAYER_DEATH" action is properly handled in both the client-side and server-side code.
- 872-919: The addition of the "PLAYER_DEATH" action is noted. Ensure that this action is properly handled in both the client-side and server-side code. Also, verify that the "Player ID" field is correctly used to identify the player who died.
Saverio976
deleted the
feature/RB-136-other-player-distroyed-when-player-die
branch
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