Revisit changes made in #661

[Poggicek]

I am all for preventing developers from stuff that will only lead to broken behavior, however in this case I don't think anything is wrong with the behavior addressed in the #661 except for not meeting developer's expectations, the original PR did not include any cases where this could be bad in any way.

the Teleport method is a general function targeted at any entity, including the controller, it's not a special method with specific behavior for each entity.

These changes have only made the code ambiguous and instead of developers learning the difference between controllers and pawns it tries to smooth out the line between them, creating only more confusion.

Please do join the discussion below this issue and say what your perspective on this is.

[Poggicek]

Extending the controller class with let's say TeleportPawn that does all the pawn sanity checks sounds a lot more sensible

edit: with the few null changes to handles, this should not even be needed thanks to optional chaining

[zonical]

Joining the discussion after talking on the Discord. I agree with @Poggicek that we should push developers to learn the differences between pawns and controllers rather than create edge-case solutions.

xstage on Discord suggested adding a warning message to Teleport (and any other functions that may need it) when run from CCSPlayerController saying something along the lines of "you may have intended to target the player pawn instead" - and direct the developer to the wiki with the appropriate documentation - maybe with an accompanying core config value disabling the warning if the developer knows what they're doing. Perhaps we could also re-arrange the article so the reference section is closer to the top and more visible to developers.

[qstage]

Maybe something like this. We will hide the Teleport method in CCSPlayerController and using the Obsolute attribute we will output a warning when compiling.

[zonical]

Maybe something like this. We will hide the Teleport method in CCSPlayerController and using the Obsolute attribute we will output a warning when compiling.

Part of me wishes there was a custom [Warning] attribute in C#, but this seems like an alright solution to me.

[Poggicek]

Maybe something like this. We will hide the Teleport method in CCSPlayerController and using the Obsolute attribute we will output a warning when compiling.

sounds good, I'd just rather use the more generic Pawn rather than PlayerPawn, there's a big difference.

[roflmuffin]

I've raised a PR reverting the functionality and adding an obsolete compiler warning to just the ccsplayercontroller class instead. I would like to avoid runtime warnings if possible. Open to adding a new TeleportPawn helper method but I think honestly people should just null chain to the pawn teleport manually and be explicit.