Closed scottnc27603 closed 9 years ago
That one section that wasn't doing anything should to be verified, but the rest looks good.
@davideaster I tested that section and could never get down to the setProperty. playerConnected is set to true by default, so this looks like older code that was no longer needed.
I thought it might be related to clients who did not respawn after being hit, or clients leaving the session and rejoining as the same player. However, I was unable to get to the playerConnected setProperty call either.
While testing last month for 0.6.23 release, I did notice that tanks are not deleted for players who simply exit out of the session (part of redmine issue #4101) - whether that is related to this chunk of code, I'm not sure.
@davideaster @allisoncorey In a perfect world we would update the application to use the new client events from the reflector. I just don't know enough about the application to easily make the changes. We could add that as a task and just go with this as a fix for now??
That section never ran before, and now it might. We don't need to change the way clients are handled--I just want to be careful not to make it worse.
Since this didn't do anything before, if you don't see what it's supposed to do, maybe it should be removed.
On Jan 8, 2015, at 11:04 AM, Scott Haynes notifications@github.com wrote:
@davideaster @allisoncorey In a perfect world we would update the application to use the new client events from the reflector. I just don't know enough about the application to easily make the changes. We could add that as a task and just go with this as a fix for now??
— Reply to this email directly or view it on GitHub.
I commented the code out, and left a comment. Opps, Looks like I id I not push that change up.
@davideaster @allisoncorey
The removal of that code is there after all.
https://github.com/virtual-world-framework/vwf/commit/500b3fd575be3ea738df8d34014b13e9cc010625
Let me know if one of you has time to review, so we can push this up to development.
Is it worth keeping the commented out section? I think I'd vote for just removing it altogether (it will still be in history if it's ever needed).
Other than that, I'm good with this.:+1: Thanks!
@allisoncorey @davideaster The findClients function was deprecated in favor of find. This branch just updates that function so that the bzflag app doesn't throw the warnings about the deprecated function.