Combine SoccerPlayer.isActiveProperty into the SoccerPlayer.phaseProperty

[samreid]

Get rid of isActiveProperty. Change poseProperty to phaseProperty. Discovered in #193

[samreid]

I'm not sure this is a win/win proposal. For instance, in SoccerPlayerNode, it is nice that the poses all have a distinct associated image. @matthew-blackman can you please inspect and advise?

[matthew-blackman]

@samreid I agree that the pose logic works well, but I still think that soccerPlayerNode can be improved by implementing a phaseProperty and moving the isActiveProperty logic into it.

For example, we could update the pose based on the phase, and also implement a screen-specific way to show/hide the players when they are done kicking in the different screens. So we'd have poseProperty and phaseProperty, but no isActiveProperty (or one that derives from phaseProperty and accounts for screen-specific logic).

Thoughts?

[samreid]

Good idea, let's do that, thanks.

[matthew-blackman]

The above commit seems to be working well aside from one bug that I'm tracking:

To address #328, in the Variability screen the soccer player should stay visible and standing after the last ball has been kicked.

The above commit achieves this for all maxKicks except for maxKicks = 30. I'm thinking this may have something to do with maxKicksLimit but I haven't deeply investigated yet. @marlitas I'll mark this help-wanted so we can do a code review and look at this synchronously.

[matthew-blackman]

This is ready for review @marlitas. Let me know if you want to discuss anything synchronously.

[marlitas]

@matthew-blackman I'm a little confused. I don't see any commits that address the bug you reported #327 (comment). Was the bug fixed in another issue or was it something else?

[matthew-blackman]

That bug was addressed in a commit in #328.

[matthew-blackman]

Reviewed with @marlitas and all looks good. Closing.