Skip to content

Add offset-reading capability for networked CUtlVector instances#1330

Merged
KyleSanderson merged 6 commits intoalliedmodders:masterfrom
nosoop:sendprop-utlvector-offs
Aug 7, 2020
Merged

Add offset-reading capability for networked CUtlVector instances#1330
KyleSanderson merged 6 commits intoalliedmodders:masterfrom
nosoop:sendprop-utlvector-offs

Conversation

@nosoop
Copy link
Contributor

@nosoop nosoop commented Aug 2, 2020

This was initially discussed in the SourceMod Discord; PR provides a way to get the actual offset of networked class members of type CUtlVector such as CBaseFlex::m_AnimOverlay and CTFPlayer::m_hMyWearables.

Tested working in TF2 with the following snippet:

public void OnPluginStart() {
	int offs = FindSendPropInfo("CTFPlayer", "m_hMyWearables");
	PrintToServer("CTFPlayer::m_hMyWearables = %04x", offs);
	
	offs = FindSendPropInfo("CTFPlayer", "m_ConditionData");
	PrintToServer("CTFPlayer::m_ConditionData = %04x", offs);
}

Not pull-ready yet; need recommendations on code style and how to deal with the CSendPropExtra_UtlVector gutted from public/dt_utlvector_send.cpp.

Heuristic per following message (the lengthproxy also holds the extradata, so I opted to use the parent table instead):

[4:19 PM] asherkin: telling that a prop is an encoded utlvector seems like the hardest part, and it looks like a simple heuristic would be that it's a table with an offset of 0, and the first child entry's name starts with lengthprop

asherkin
asherkin reviewed Aug 3, 2020
View reviewed changes
Copy link
Member

@asherkin asherkin left a comment

Choose a reason for hiding this comment

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

This looks great! It is a perfect first step towards supporting these props natively.

Not pull-ready yet; need recommendations on code style and how to deal with the CSendPropExtra_UtlVector gutted from public/dt_utlvector_send.cpp.

I'd say the HACK_CSendPropExtra_UtlVector struct here is a fine solution - this branch is highly unlikely to be hit in an engine that doesn't support CUtlVector props, and should only be reached for actual CUtlVectors, so won't have random fallout even if broken. It might be worth a quick check in CS:GO to make sure the struct matches there.

@nosoop
Copy link
Contributor Author

nosoop commented Aug 3, 2020

A disassembly of SendPropUtlVector suggests that CSendPropExtra_UtlVector's size and assigned member orders match between CS:GO and TF2, so I'd imagine it's fine.

FindSendPropInfo built against this PR reports that CBaseFlex::m_AnimOverlay is at 0x4EC for CS:GO, which seems right since it shows up in CBaseAnimatingOverlay::StudioFrameAdvance, and TF2 reports the offset found in its own function.

I'll go ahead and set this up for review since it appears to clear both games.

@nosoop nosoop marked this pull request as ready for review August 3, 2020 18:36
@psychonic
Copy link
Member

psychonic commented Aug 3, 2020

Do we even need the whole struct? It looks like only one member is used. If there's any chance that this could different across engine variables, why not just put its offset in gamedata?

@nosoop
Copy link
Contributor Author

nosoop commented Aug 4, 2020

I don't have an opinion for / against putting it in gamedata, but I'd imagine we could also use CSendPropExtra_UtlVector::m_ElementStride in the future should there be a desire to access CUtlVector elements via natives; the other four members would remain unused.

No problems switching it over to gamedata if that's the consensus.

@asherkin
Copy link
Member

asherkin commented Aug 4, 2020

Using gamedata makes sense 👍

KyleSanderson
KyleSanderson requested changes Aug 4, 2020
View reviewed changes
Copy link
Member

@KyleSanderson KyleSanderson left a comment

Choose a reason for hiding this comment

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

This looks good, nice work man. When it's ready for review (after the offsets stuff comes in - which is a good suggestion) r? me again and I'll take another look for style and things like that.

@nosoop
Copy link
Contributor Author

nosoop commented Aug 4, 2020

Moved it over to offsets as requested. I'm not sure where in gamedata/core.games/ the offset should go in; at the moment I've tentatively put the following in common.games (currently uncommitted):

"SendPropExtraVecOffset"
{
	"windows"		"16"
	"linux"			"16"
}

Let me know if I should add it into common.games or all the engine ones.

@asherkin
Copy link
Member

asherkin commented Aug 5, 2020

common.games makes sense to me - even stuff that's just the same for a handful of engines lives in there currently. I'd probably be inclined to name it after the struct/member to make it more obvious when updating it later / adding the stride - just CSendPropExtra_UtlVector::m_Offset?

@nosoop
Copy link
Contributor Author

nosoop commented Aug 6, 2020

Done. Class::m_Member's my go-to for gamedata naming; just didn't want to stray from existing conventions here.

@nosoop nosoop requested a review from KyleSanderson August 6, 2020 03:49
@KyleSanderson KyleSanderson merged commit c5619f8 into alliedmodders:master Aug 7, 2020
@KyleSanderson
Copy link
Member

KyleSanderson commented Aug 7, 2020

Nice work on this one - didn't want to nit you on this one @nosoop so made a tiny commit and landed. Thanks for the contribution!

@nosoop nosoop deleted the sendprop-utlvector-offs branch August 7, 2020 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asherkin asherkin asherkin left review comments

@KyleSanderson KyleSanderson KyleSanderson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nosoop @psychonic @asherkin @KyleSanderson