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

add: ET protocol 84 and 284 (ETTV) #2

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

ryzyk-krzysiek
Copy link

@ryzyk-krzysiek ryzyk-krzysiek commented Aug 9, 2024

Hello,

Some time ago I've added ET protocol support 84 and 284 (ETTV also known as tv 84) but I don't think it's 100% finished or would be up to your standard (for example I've changed some stuff where according to your comments it is an issue (API stuff)). And so I thought about opening PR as draft only (if it's not an issue) in case maybe you/someone would be interested in it and finishing it (I don't think I will unless you have interest in looking into it and letting me know what I should do, but I can't promise I would do it anyway) or just using it.

What I added more specifically:

  • added protocol 84 and 284 demo parsing
  • added ET legacy mod constants, some overlap with other mods but not everything
  • added ET configstring parsing (not 100% sure it behaves correctly I didn't do much testing here other than it works)
  • added ET obituaries parsing, uses legacy mod meansofdeath which is a bit different for all other mods pretty much
  • added ET scores parsing
  • added ET ws parsing legacy/etpro/other
  • added ET sc parsing legacy/etpro/other
  • added ET chat parsing legacy/etpro/other
  • protocol 284 parsing required some additional stuff for base parser as it differes a bit than all other protocols (and the logic might be a bit scuffed)
  • added conversion from 284 protocol to 84 by cutting by time to a specific players PoV
  • added to GUI a dropdown of possible conversions from 284 to 84 (dropdown shows the name of the player and times where it will be cut so if someone wants the cut to be more specific, first convert to 84 and then use other already existing features)
  • as a note most etpro mod server commands are encoded and because as far I know they tried to reduce bandwidth they changed what fields have in them (at least for some, see obituaries parsing) compared to other mods
  • changed some API stuff, like increased UDT_PLAYER_STATS_MASK_BYTE_COUNT from 40 to 50, also see this https://github.com/mightycow/uberdemotools/pull/2/files#diff-24ee4628e8f257ce90821ffd08be109419d57ecdb6e23cd87fb4b5ab92655326R72

- only really tested chat and obituaries which seem to work fine as of now
- very little actual changes made just spent most of the time adding data tables
- protocol 284 doesn't work since it needs additional parsing
add demo conversion from protocol 284 to 84
add configstring processing for ET protocols
add chat processing for ETPro mod
add obituaries processing for ETPro mod
add scores processing for ET (excluding ETPro since it's commands are encoded)
add demo conversion from protocol 284 to 84 to GUI

Conversion should be working fine for both ETPro tv demos and ETL tv. There could be some bugs (I know of one in ETL but it's Legacy mod issue that needs fixing).
@mittermichal
Copy link

got SIGSEGV when cutting this file:
gross-broken-z.zip
udt_cutter t -s=10 -e=10000000 demo_files/tv_84/gross-broken-z.tv_84

udtBaseParser::WriteGameState parser.cpp:1085
udtBaseParser::WriteFirstMessage parser.cpp:383
udtBaseParser::ParseServerMessage parser.cpp:313
udtBaseParser::ParseNextMessage parser.cpp:142
udtParserRunner::ParseNextMessage parser_runner.cpp:84
RunParser utils.cpp:368
udtCutDemoFileByTime api.cpp:984
CutByTime app_demo_cutter.cpp:794
udt_main app_demo_cutter.cpp:1011
main shared.cpp:207

using -s=10 -e=20 outputs empty dm_84

is there a way to specify POV for tv_84 demo ?

@ryzyk-krzysiek
Copy link
Author

ryzyk-krzysiek commented Aug 23, 2024

@mittermichal

Only from GUI and it is limited to whole sections. For example a player was in the tv demo from 1min to 3min and 4min to 6min, so you can choose only those 2 specific cuts. Obviously it is something that could be changed and what I mentioned in the PR that if someone wants to more specific cut then first it needs to be converted to dm_84 with chosen section and then can be cut from again using already existing features. I did it that way because it was easiest and fastest way to do it which makes some sense and if needed more sophisticated method could be developed (mine is rather proof of concept).

udt_cutter in current implementation is not appropriate, UDT_converter is appropriate but it lacks the ability to read new arguments that need to be passed and proper udtProtocolConversionArg created (and missing IsValidConversion changes for ET protocol). Just adding both of these might not be enough, because there probably will be missing check for what if the specifed times go beyond the time where playerstate for wanted player exist (inputing to early time should be fine tho). In such case demo might even still play but will be showing "garbage" as there will be missing playerstate in snapshots.

@mittermichal
Copy link

mittermichal commented Aug 24, 2024

yeah it was easy to modify UDT_converter to try tv_84 ->dm_84
so far I found 2 slight issues:

  1. the entity of playerstate is present which look like this in freecam or thirdcam (also when you just die and see thirdcam of your body)
    shot0000
    this should fixable by removing corresponding ps entity (done this in anders.gaming.libtech3) or moving it somewhere far I guess.

  2. both artillery and airstrike announcement that are meant only for the player that called them are heard globally. This I have fixed in libtech3 with this logic:
    if (eventType==127 or eventType==128) convertPOVnumber!=new_ps->sharedState.singleClient:
    mark_entity_as_removed()

dunno if its correct. the condition "convertPOVnumber!=new_ps->sharedState.singleClient" doesn't really makes sense to me. It was long time ago I did that and didn't left any comment in the code

@ryzyk-krzysiek
Copy link
Author

  1. Didn't know it caused this, I remember thinking about removing it, but I don't remember why I didn't do it in the end, I might have forgotten because while I was adding conversion the clientnum was still just hardcoded so I couldn't access it. Anyway, I think it should be removed as entitystate of self is not sent over network and cgame currentState is created from playerstate with BG_PlayerStateToEntityState.
    Something like this fixes the bug but Im not 100% sure right now if comparing clientnum with entitynumber could have some edge cases where it could be wrong:
diff --git a/UDT_DLL/src/protocol_conversion.cpp b/UDT_DLL/src/protocol_conversion.cpp
index 2780789..bac47d1 100644
--- a/UDT_DLL/src/protocol_conversion.cpp
+++ b/UDT_DLL/src/protocol_conversion.cpp
@@ -672,8 +672,16 @@ void udtProtocolConverter284to84::ConvertEntityState(idLargestEntityState& outEn
 {
 	idEntityState84& out = (idEntityState84&)outEntityState;
 	idEntityState84& in = (idEntityState84&)inEntityState;
-	out = in;
-	(idEntityStateBase&)outEntityState = inEntityState;
+	
+	if ((s32)ConversionInfo->ClientNum != inEntityState.number)
+	{
+		out = in;
+		(idEntityStateBase&)outEntityState = inEntityState;
+	}
+	else
+	{
+		Com_Memset(&outEntityState, 0, sizeof(outEntityState));
+	}
 }
  1. I was aware of that issue, it is also present in the etpro tvdemo example I added. I've never tested if it happens in legacy mod and never tried figuring out why it even happens in etpro.

In etpro also fireteam and scoreboard are broken both related to hp missing (both show as if players are dead always), never tried figuring out why though. IIRC in legacy it works.

Another thing similiar to entitystate not being removed now is that commands not meant for a client are also not removed, for legacy it is just boring work of going through a list of commands that exist and adding to the already existing removal (udtProtocolConverter284to84::ConvertCommand). ETPro on the other hand though is a bigger challenge because they are 1. encoded 2. they are bitstreams (etpro basically added MSG functions to mod and used them for commands), so filtering them is not as trivial.

@ryzyk-krzysiek
Copy link
Author

ryzyk-krzysiek commented Aug 25, 2024

I just realised why I didn't add the above entity removal before, because it doesn't remove it but makes another entity 0 essentially empty, so a player with clientnum 0 is a ghost. I don't think there is a way in UDT to actually remove entity tho, or at least I couldn't figure it out last time. I think technically entity either should be never written (skip the write call) or set to null (in udtProtocolConverter284to84::ConvertEntityState), but the way UDT works right now you can't do neither.

@mittermichal
Copy link

mittermichal commented Aug 26, 2024

isn't this

// a NULL to is a delta remove message
if(to == NULL)
{
if(from == NULL)
{
return ValidState();
}
WriteBits(from->number, GENTITYNUM_BITS);
WriteBits(1, 1);
return ValidState();
}
removing the entity?
also

if ((s32)ConversionInfo->ClientNum == inEntityState.number)
{
	outEntityState.eFlags |= 0x00000040; // ETPro and ET:legacy NODRAW flag
}

does the job too 😄

@ryzyk-krzysiek
Copy link
Author

Yes, I think it removes it, so 1. it might break parsing or cause other unwanted consequences 2. It still writes something to demo, which doesn't make it perfect solution anyway.

That's a nice idea with the nodraw flag. I guess indeed the easiest solution would be to convert the entity to something that will not get processed in CG_ProcessEntity at all, which could be done in a few ways. Still not perfect solution in my mind as the entity shouldn't be present in demo at all, but good enough since it doesn't require changing UDT itself to allow not writing an entity into a demo.

I wonder if the entity being present in dm84 with just nodraw flag could interfere with some UDT analyzers, Im not familiar with them at all really.

@triwats
Copy link

triwats commented Aug 28, 2024

This would be a really wonderful addition to this project and offer support for a large user base. Would love to see it make it into UDT!

@mittermichal
Copy link

mittermichal commented Aug 29, 2024

what if the specifed times go beyond the time where playerstate for wanted player exist (inputing to early time should be fine tho). In such case demo might even still play but will be showing "garbage" as there will be missing playerstate in snapshots.

just tested what happens with tv_84 conversion if players reconnect and converted demo playback stopped after disconnect.

reconnect.zip

@ryzyk-krzysiek
Copy link
Author

ryzyk-krzysiek commented Aug 30, 2024

what if the specifed times go beyond the time where playerstate for wanted player exist (inputing to early time should be fine tho). In such case demo might even still play but will be showing "garbage" as there will be missing playerstate in snapshots.

just tested what happens with tv_84 conversion if players reconnect and converted demo playback stopped after disconnect.

reconnect.zip

I looked at it briefly on the included etpro tv demo in the PR and it might feel like it stopped because when there is no valid tv player playerstate then snapshot is not written (snapshot contains playerstate and entities), but for example server commands are still written and will be read in game, it's just fast because there is nothing to render I guess.

Easy way to test is to just look at demo file size after cut, when going beyond possible times (starting too soon or ending too late causes the file to be bigger than doing correct start and end times). Imo this is not acceptable not even from file size reasons but because you save a lot of server commands that are not relevant (it might be irrelevant for playing back the demo, but not really if you want to analyze it again with UDT as it will show commands from beyond when player existed).

It seems it also "breaks" UDT parsing of such cut demo, a lot of warnings about WARNING: udtBaseParser::ParseSnapshot: Delta frame 1 too old.

@puran-superboyy
Copy link

Just wanted to say I can't wait to use this scanner for about 3 years worth of ET Legacy demos :)

Scanning for headshots would be a nice feature as well, as a 6 headshot double kill is much better than a triple kill with bodyshots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants