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 TF2_SetClientTeam to provide symmetry to TF2_GetClientTeam #267

Merged
merged 3 commits into from Feb 24, 2015
Merged

Add TF2_SetClientTeam to provide symmetry to TF2_GetClientTeam #267

merged 3 commits into from Feb 24, 2015

Conversation

50Wliu
Copy link
Contributor

@50Wliu 50Wliu commented Feb 21, 2015

Continues #201, which added TF2_GetClientTeam. This PR adds TF2_SetClientTeam in order to provide full symmetry. I was thinking about doing TF2_ChangeClientTeam, since it wraps around ChangeClientTeam, but I decided against it because of TF2_{Get|Set}PlayerClass.

Was also thinking about updating tf2_stocks.inc to new-style syntax but I have no idea how to do the C++ side of methodmaps.

And...removes some trailing whitespace.

@KyleSanderson
Copy link
Member

And...removes some trailing whitespace.

Definitely include that in a different commit then the one where you've added something.

@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 21, 2015

Thanks, will definitely remember that for next time.

@psychonic
Copy link
Member

I don't know if others agree, but I'd prefer it as TF2_ChangeClientTeam, to keep the name similar to the current API. "Set" can also imply that it's just setting a value, rather than taking action (and changing a clients team does do more behind the scenes than flipping a var).

@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 22, 2015

I've posted a poll in the Scripting subforum-I guess we'll look and see after a while.

@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 23, 2015

So it's pretty obvious which one's going to win at this point. Changed.

In addition, Powerlord pointed out that there's a new CTFPlayer::ChangeTeam( int iTeamNum, bool bAutoTeam, bool bSilent );, so I guess if that could be implemented in Sourcemod, that would be great.

@psychonic
Copy link
Member

In addition, Powerlord pointed out that there's a new CTFPlayer::ChangeTeam( int iTeamNum, bool bAutoTeam, bool bSilent );

If by new, you mean eight years old 😉. It's existed since the Orangebox engine debuted.

The beauty of the one we're using now is that it is on a fairly stable interface. The other one would require gamedata as it would occasionally break. Additionally, automatically selecting a team and blocking the player_team event can be done manually without much hassle.

@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 23, 2015

New™!
I've been working fine with ChangeClientTeam and AddCommandListenering jointeam, so I guess it's ok then ;P.

Also, the Travis CI build seems to have timed out 😢.

@psychonic
Copy link
Member

Also, the Travis CI build seems to have timed out 😢.

I have convinced it to reconsider...

@psychonic psychonic mentioned this pull request Feb 24, 2015
psychonic added a commit that referenced this pull request Feb 24, 2015
Add TF2_SetClientTeam to provide symmetry to TF2_GetClientTeam (r=psychonic).
@psychonic psychonic merged commit b8223d1 into alliedmodders:master Feb 24, 2015
@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 25, 2015

Is there any chance this can be merged into the 1.7-dev branch, or will this have to wait until 1.8?

@KyleSanderson
Copy link
Member

Do you have a more compressed changeset with just the changes you need to support this feature?

@asherkin
Copy link
Member

It'll probably get picked up in @psychonic's next cherry pick run, generally stuff gets left to bake in master for a few days.

psychonic added a commit that referenced this pull request Feb 26, 2015
Add TF2_SetClientTeam to provide symmetry to TF2_GetClientTeam (r=psychonic).
@50Wliu 50Wliu deleted the tf2_setclientteam branch June 7, 2015 21:10
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.

None yet

4 participants