akmdm 10518 makes private messages between players appear in the server logs? That sounds problematic. Did something happen so that you think this is necessary?
I've was recently thinking about how I originally wanted to prevent private messages between two players from being logged, and wondered if it was really a good idea to be hiding those messages from the server host. I believe that since the server is an authority figure, a host should have some right to see what kinds of messages players are sending to each other. To me, it's more important that other players can't see what the two are sending to each other. This is my view on the issue, which people may or may not like if they knew about it, but we can discuss this more on #zadev.
In addition, since the server has to receive and/or send every private message, a server host can create a custom binary for themselves that still logs private messages between two players anyways, and would therefore be exploitable.
akmdm)
I added 10517-10522.
10535 has as a small (more or less cosmetic issue): CREATETRANSLATION_PALETTE, CREATETRANSLATION_RGB and CREATETRANSLATION_DESATURATED are mutually exclusive, but handling them as flags allows to select any combination of them. I think it would be nicer to stick to the original bool logic and add the bools with "addBit". So, first "addBit" to encode "bIsTypeTwo" (in the old notation). If that is true, you can add twp more "addBit"s, one for "bIsDesaturated" and one for "bIsEdited".
akmdm akmdm akmdm I uploaded another round of patches, including 10535_v2.patch which touches up on the original 10535.patch. I also merged all "SetPlayerStatus" server commands into one single command to further reduce the number of SVC_ commands we have, it should also be sending one less byte per call than the older commands.
akmdm akmdm akmdm akmdm Thanks for the updated patches! I'm currently going through them and will report here as I go.
10535_v2.patch:
- The client is always reading the three bits bIsEdited, bIsTypeTwo and bIsDesaturated, but the server is not writing the second two in void SERVERCOMMANDS_CreateTranslation( ULONG ulTranslation, ULONG ulStart, ULONG ulEnd, ULONG ulPal1, ULONG ulPal2, ULONG ulPlayerExtra, ServerCommandFlags flags ).
- bIsDesaturated is not needed in case bIsTypeTwo is false. This doesn't cost any extra bandwidth, but may be confusing.
The other new patches all look good, so I added them.
Also added 10552+3.
BTW: 10518 is not in yet. See my older comment on this.
akmdm Reply To torr_samaho
BTW: 10518 is not in yet. See my older comment on this.
I think it's okay to let go of 10518.patch. I'll stick back to my original idea on redacting private messages between two players from the server's log. If a server host really wants to peek on the private messages players send each other, they can create their own custom binary for that purpose.
BTW: I'm uploading 10535_v3.patch. I believe the boolean bit logic you suggested earlier will become problematic and more confusing in the long run when we eventually port the new translation types: blended and tinted, from GZDoom. We'll need to keep track of five different translation types when that happens. Therefore I switched back to using enumerations which will simplify a lot of things for us. bIsEdited is still a bit, while the translation type occupies seven bits, so no extra bandwidth is required.
akmdm I just uploaded another batch of patches for review. I think I have covered everything I wanted to address for Zandronum 3.1 at this point, unless I'm missing anything. Other issues/features I would like to postpone for a possible 3.2 release. Some things worth noting:
10558: A lot of the underlying code surrounding the Printf calls could still use some cleanup. I'd probably save this post-3.1 unless it's important to do right now.
10559: Especially now that "new text colours" (260+ colours) is officially supported in 3.1, sending color as a byte won't be enough in some cases, and I couldn't find a convenient way of sending it as a variable type to optimize the change. This issue got brought up to me by somebody who was printing ACS HUD messages using custom colours and reported the wrong colour was being used in online games.
Fortunately, when I originally changed SERVERCOMMANDS_PrintHUDMessage, I added optimizations to conserve the amount of bandwidth consumed, so changing the type to a short probably won't hurt too badly compared to 3.0.
akmdm)
Thanks for the patches! Quick comment on "TEAM_GetTextColorName": I think returning NULL for invalid teams is very dangerous, since the effect of feeding NULL to "printf %s" is AFAIK undefined and thus may cause trouble.
10562: This does not save the position in the messages RingBuffer, just the messages.
EDIT: I added all patches except for 10558 and 10562.
akmdm I updated 10558, so now it returns "Untranslated" if either the team's invalid or it doesn't have a text colour specified.
That's a good solution. Thanks! I added 10558_v2.
Thanks for the updated patch! I added 10562_v2.
akmdm 
Fork
Just a bunch of general patches I had saved that address at least several different issues.