CnCNet / xna-cncnet-client

XNA / MonoGame based client for playing classic Command & Conquer games both online and offline with a CnCNet game spawner.
Other
228 stars 91 forks source link

#301 make player list box reusable and fix list update issues #322

Closed devo1929 closed 2 years ago

devo1929 commented 2 years ago

This makes the PlayerListBox that is used in the CncnetLobby reusable so that it can then be reused in the Private Messaging Window.

It also resolves some issues in "updating" the lists for various actions including: user added, removed, friended, ignored, etc... When these actions occur, this list is updated accordingly where ever it is used and in the same way in all locations.

Rampastring commented 2 years ago

A word of warning related to performance. Refreshing of the player list box is a performance-critical operation for games with large player bases like YR and MO. Back when YR hit 5000 concurrent players in May 2020, the performance of the implementation turned out to be insufficient, which was causing the client to stall on low-performance machines and YR players were even getting kicked from the IRC network due to the client processing messages too slowly.

The current public implementation with SortedUserCollection and refreshing the list directly based on it was written back then and helped YR survive the surge in players. This suggested implementation might (not necessarily saying it will, but it's necessary to keep an eye on it) be heavier with heavy use of ToList (creating new list objects), LINQ sorting and selection operations and, based on a quick look and assuming I haven't missed anything, re-sorting the users in the player list each time any user info is updated.

I support re-using code so sharing the player list box between the lobby and the PM window is a good idea to explore, and doing so obviously needs changes to the player list class as well. But I'm afraid this implementation will backfire if there's a new user spike in the future. For regular lobby channels, it could be performance-friendlier to instead edit the user comparison function given to SortedUserCollection.

devo1929 commented 2 years ago

@Rampastring Yea, I had the same concern, but I wasn't sure if it was completely warranted. You confirmed it, though. I want to do a little more of an overhaul then. I'll close this for now.