Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CLIENT-NETWORK: Add system receive broadcast position absolute #104

Merged

Conversation

romainpanno
Copy link
Collaborator

@romainpanno romainpanno commented Oct 15, 2023

MINOR

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

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

  • Other information:

Summary by CodeRabbit

  • New Feature: Introduced real-time player position updates. This enhancement allows the system to receive and process broadcasted absolute position messages, updating the position of other players in the game in real-time. This feature enhances the multiplayer experience by providing accurate and up-to-date positioning information.
  • Improvement: Enhanced thread safety when accessing player data. This update ensures that the game's performance remains stable and reliable, even when multiple processes are accessing and updating player positions simultaneously.

@romainpanno romainpanno added the enhancement New feature or request label Oct 15, 2023
@romainpanno romainpanno self-assigned this Oct 15, 2023
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2023

Walkthrough

The changes primarily focus on introducing a new function receiveBroadcastAbsolutePosition to handle broadcasted absolute position messages. This function updates the position of other player entities in the registry in a thread-safe manner. The changes also include an update to the packet structure used for broadcasting position data.

Changes

File Path Summary
src/Client/Systems/Network/ClientNetwork.cpp Introduced a new function receiveBroadcastAbsolutePosition to handle broadcasted absolute position messages and update the position of other player entities in the registry.
src/Client/Systems/Network/ClientNetwork.hpp Added the declaration of the new function receiveBroadcastAbsolutePosition.
src/Nitwork/NitworkClient.hpp Added a new entry to the messageHandlers map to handle POSITION_ABSOLUTE_BROADCAST message type.
src/Nitwork/NitworkServer.hpp Updated the function call to use a different packet structure for broadcasting position absolute data.

🐇

"In the land of code, where the shadows lie,

A rabbit hops, under the cloudless sky.

With each key press, a new function born,

To track the players, in the world they adorn.

Thread-safe and secure, the registry stands,

Updated with care, by coderabbit's hands.

So raise your cups, for the code's new lease,

And celebrate the changes, in the land of peace." 🥂🎉


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between de2ff84 and a36c457.
Files selected for processing (4)
  • src/Client/Systems/Network/ClientNetwork.cpp (1 hunks)
  • src/Client/Systems/Network/ClientNetwork.hpp (1 hunks)
  • src/Nitwork/NitworkClient.hpp (1 hunks)
  • src/Nitwork/NitworkServer.hpp (1 hunks)
Files skipped from review due to trivial changes (2)
  • src/Client/Systems/Network/ClientNetwork.hpp
  • src/Nitwork/NitworkClient.hpp
Additional comments (Suppressed): 2
src/Nitwork/NitworkServer.hpp (1)
  • 161-165: The function call sendData<struct packetPositionAbsolute_s>(packet); has been replaced with sendData<struct packetPositionAbsoluteBroadcast_s>(packet);. Ensure that the new struct packetPositionAbsoluteBroadcast_s is defined and compatible with the sendData function. Also, verify that all instances where POSITION_ABSOLUTE_BROADCAST messages are sent have been updated to use the new packet structure.
src/Client/Systems/Network/ClientNetwork.cpp (1)
  • 169-192: The new function receiveBroadcastAbsolutePosition is introduced to handle the receipt of broadcasted absolute position messages. It uses a mutex to ensure thread safety when accessing the shared Registry object. The function looks for the corresponding player entity in the registry and updates its position. If the entity is not found, the function returns early. This function seems to be well-structured and follows good practices for thread safety and error handling. However, it's important to ensure that the mutex lock doesn't introduce any performance issues or deadlocks.

src/Client/Systems/Network/ClientNetwork.cpp Show resolved Hide resolved
@Saverio976 Saverio976 merged commit 09df31e into dev Oct 15, 2023
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between a36c457 and bc2ccb6.
Files selected for processing (1)
  • src/Client/Systems/Network/ClientNetwork.cpp (1 hunks)
Additional comments (Suppressed): 2
src/Client/Systems/Network/ClientNetwork.cpp (2)
  • 169-191: The receiveBroadcastAbsolutePosition function is well-structured and follows good practices. It uses a mutex to ensure thread safety when accessing the registry, which is crucial in a multi-threaded environment. The function also checks if the other player entity is found in the registry before updating its position, which is a good practice to avoid potential errors. However, it's important to ensure that the msgPositionAbsoluteBroadcast_s structure and msg object are correctly defined and populated before they are used.

  • 193-195: The getNetworkSystems function is not updated to include the new receiveBroadcastAbsolutePosition function. As per the previous comment chain, this is intentional and does not need to be changed.

@Saverio976 Saverio976 deleted the feature/RB-126-system-receive-broadcast-position-absolute branch October 17, 2023 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants