Skip to content

Conversation

@Pirulax
Copy link
Contributor

@Pirulax Pirulax commented Nov 3, 2020

It was a very outdated code.
Also, I can't belive we've used it like this, its horrible.. Heap allocating and copying vectors, and maps for every packet?
This PR fixes it all! At once. (Coudnt really break it into more PRs).

Features:

  • Added static BroadcastIf - Send the packet only to a player for which predicate is true
  • BroadcastJoinedIf - A member function that uses the internally stored and optimized m_JoinedByBitStreamVer for BroadcastIf
  • The above mentioned function can be used like as a base for other existing functions, for example: BroadcastJoinedDimension (I've refactored all of them to use BroadcastJoinedIf)
  • The former 2 give a pretty good performance boost. Not tested, but it would make sense, especially on servers where there are a lot of packets sent / frame. (No heap allocs anymore, and sorts, so :D)
  • Refactored the code to c++17 standards - Now its even more readable
  • Added 2 todos!! Maybe even more!
  • Added noexcept and const wherever possible
  • Cleaned up old code related to the manager (eg.: Use GetAllPlayers() instead of IterBegin and IterEnd, and use IterateJoined where the old code used IterBegin and IterEnd and checked if the player is joined)
  • Moved CSendList from CGame to its own file, because in the process of refactoring it ended up being quite big
  • Added GetNetServerReliability and GetNetServerPriority to CPacket. These functions are wrappers and translator of GetFlags (They translate from our enum to raknet enums)

When reviewing

  • See if the old codes logic matches the new one. Eg.: In commit bdbb722 see if I've missed a check we copy pasting code, or something. (I didnt)

@qaisjp Sorry for some fuckups in the commits, but coudn't properly select staged in vsc.
I've tried my best to make the commits atomic.

Comment on lines +164 to +165
if (pPlayer->IsBeingDeleted())
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this check, because I presume it was unintentionally left out?

@Pirulax Pirulax force-pushed the refactor/playermgr branch 2 times, most recently from b160ec8 to be931c6 Compare November 4, 2020 01:49
@Pirulax
Copy link
Contributor Author

Pirulax commented Nov 10, 2020

Me when i press Build: https://imgur.com/gallery/b4GHdOO

@Pirulax Pirulax marked this pull request as draft January 30, 2021 02:08
@Pirulax
Copy link
Contributor Author

Pirulax commented Jan 31, 2021

Requires some testing.
Would be nice if we could gather 10-20 players.
One issue that might occur is that any of the containers (m_Players, m_SocketPlayerMap, m_JoinedByBitStreamVer) gets modified while iterating thru them.
It's very unlikely tho.

@botder botder added this to the Spring Maintenance milestone Feb 3, 2021
Copy link
Contributor Author

@Pirulax Pirulax left a comment

Choose a reason for hiding this comment

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

Please help me out here.
Nvm oof, that didn't work the way I imagined it.

@Pirulax Pirulax marked this pull request as ready for review March 22, 2021 07:59
@patrikjuvonen patrikjuvonen marked this pull request as draft April 8, 2023 10:45
@patrikjuvonen patrikjuvonen added the feedback Further information is requested label Apr 8, 2023
@patrikjuvonen patrikjuvonen removed this from the Spring Maintenance milestone Apr 8, 2023
@patrikjuvonen
Copy link
Contributor

Merge conflicts must be resolved.

@github-actions github-actions bot added the stale Inactive for over 90 days, to be closed label Jul 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

This draft pull request is stale because it has been open for at least 90 days with no activity. Please continue on your draft pull request or it will be closed in 30 days automatically.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

This draft pull request was closed because it has been marked stale for 30 days with no activity.

@github-actions github-actions bot closed this Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback Further information is requested refactor stale Inactive for over 90 days, to be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants