search

SmartlyDressedGames / Legally-Distinct-Missile

Fork of Rocket for Unturned maintained by the game developers.
MIT License
77 stars 29 forks source link

UnturnedPlayer factory methods #26

Open Game4Freak opened 4 years ago

Game4Freak commented 4 years ago

I think most plugin devs use the UnturnedPlayer class because it provides multiple shortcuts that are very handy. But most of the factory methods dont work as expected and can result in many issues. This has been a issue in RocketMod too for a long long time. UnturnedPlayer.FromName only returns a valid UnturnedPlayer object if an actual player is found (how it should work). But UnturnedPlayer.FromCSteamID will always return a valid UnturnedPlayer object if the CSteamID isnt null or empty or 0. That means that you will get a object back even if no actual player exists. When you use that object it will throw error out like nothing because it's player object is null. https://github.com/SmartlyDressedGames/Legally-Distinct-Missile/blob/master/Rocket.Unturned/Player/UnturnedPlayer.cs#L170 So most of the time you have to check if your UnturnedPlayer object is null and if the UnturnedPlayer.Player object is null. I think that UnturnedPlayer.FromCSteamID should only return a valid UnturnedPlayer object if an actual player is found.

Game4Freak commented 4 years ago

Also UnturnedPlayer.FromSteamPlayer and UnturnedPlayer.FromPlayer will have similar issues when you pass null to them

SDGNelson commented 4 years ago

On the one hand I am hesitant to change the behavior of existing Rocket code, but I have seen a lot of exceptions related to the unusual way those method work. Your proposal seems reasonable to me, and returning null in those cases is how I would expect them to work.

Would anyone else like to weigh in on this or voice their thoughts?