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

LMSServer.show_players_sync_status() #5

Open
Flimbo99 opened this issue Apr 7, 2017 · 9 comments
Open

LMSServer.show_players_sync_status() #5

Flimbo99 opened this issue Apr 7, 2017 · 9 comments

Comments

@Flimbo99
Copy link

Flimbo99 commented Apr 7, 2017

why wouldn't this return a list of player instances rather then the names and MACs?
and could player.get_synced_players() (optionally) also return the number of the sync group, assuming that is a static number.

@elParaguayo
Copy link
Owner

Why would it return the players? The method shows which players are synced to which other players. There are already methods to get players from the server and I don't see this being one of the primary ways of doing so. I'm happy to be convinced otherwise though!

It's certainly trivial to make this return player instances (I think I'd do this via an option as I don't see a reason for it to be default).

As for your second question, I don't think there is a static number for the group. This method returns an index (based on the groups returned by the server) and is basically just there so you can see which players are in a group together. A group number is really meaningless.

elParaguayo added a commit that referenced this issue Apr 7, 2017
elParaguayo added a commit that referenced this issue Apr 7, 2017
@elParaguayo
Copy link
Owner

@Flimbo99
Copy link
Author

Flimbo99 commented Apr 7, 2017

I am probably just thinking out loud.
To make e.g. a GUI, I would have to define a GUIplayer class that such that each instance has a reference to the LMSPlayer instance that is representing. So how one GUIplayer instance figures out which other GUIplayer instance should appear synchronised to it could be done by finding the ones with synchornised LMSPlayer instances. Having only the MACs makes that a bit harder.
I am also a bit confused about how the identity between LMSplayer instances is checked. LMSplayer.get_synced_players(get_players=True) does return LMSplayer instances, but they are not the same as those returned by LMSServer.get_players().
LMSServer.get_players() would also return different instances each time it is called. I don't see an obvious way to avoid that either since player may suddenly disappear, then reappear. It should be up to the (G)UI to handle that. Then I wonder why the LMSServer should ever return any LMSPlayer instances at all, and the same for LMSPlayer. Which basically makes your point.

@Flimbo99
Copy link
Author

Flimbo99 commented Apr 7, 2017

In a GUI, the player would be uniquely identified by server:port + player MAC. A GUIPlayer instance should check if a matching LMSPlayer instance can be created (i.e. if the player is connected at that moment) or change the way it appears if it can't. It should hang on to that LMSPlayer instance until connection is lost, at which point it could release the instance (or not). Between sessions of the GUI, you may also want to save the identifiers for the players (e.g. to remember which ones to display or hide).
Again, making the point that having LMSServer and LMSPlayer ever returning any LMSPlayer instances probably doesn't make much sense.

At any rate, thank you for your effort on this!

@elParaguayo
Copy link
Owner

As far as I was aware, players are identified just by the MAC address. That's why you can test equivalence of player instances and it looks at the MAC address (you can test equivalence by comparing two LMSPlayers or one LMSPlayer with a MAC address).

If that's wrong then I'll happily amend the code. I'm no expert when it comes to LMS so I'm very happy to be corrected as to how it works and fix my code accordingly!

@elParaguayo
Copy link
Owner

There was a little bug in the player equivalence test which I've fixed in development branch.

This is what I mean by the players being the same:

>>> players = server.get_players()
>>> players
[LMSPlayer: elParaguayo's Laptop (41:41:41:41:41:41)]
>>> laptop = players[0]
>>> laptop
LMSPlayer: elParaguayo's Laptop (41:41:41:41:41:41)
>>> synced = server.show_players_sync_status()
>>> synced
{'players': [{'sync_index': -1, 'ref': u'41:41:41:41:41:41', 'name': u"elParaguayo's Laptop"}], 'group_count': 0, 'player_count': 1}
>>> synced["players"][1]["ref"] == laptop
True
>>> synced_players = server.show_players_sync_status(get_players=True)
>>> synced_players
{'players': [{'player': LMSPlayer: elParaguayo's Laptop (c8:ff:28:b5:ea:b5), 'sync_index': -1, 'ref': u'41:41:41:41:41:41', 'name': u"elParaguayo's Laptop"}], 'group_count': 0, 'player_count': 1}
>>> synced_players["players"][1]["player"]
LMSPlayer: elParaguayo's Laptop (41:41:41:41:41:41)
>>> synced_players["players"][1]["player"] == laptop
True

Is that enough for your GUI to tell that the players are the same?

@Flimbo99
Copy link
Author

Flimbo99 commented Apr 9, 2017

An instance that is equivalent but not equal doesn't work. But now I understand that LMSTools can not really keep track of its own instances; that is (and probably should be) up to wherever they are used.

@elParaguayo
Copy link
Owner

Can you explain why it doesn't work? If it's something vital, I'll consider amending the code.

@elParaguayo
Copy link
Owner

elParaguayo commented Apr 10, 2017

One way to deal with this could be to create an object that maintains a pool of LMSPlayers. This object should listen for the CLIENT_NEW, CLIENT_DISCONNECT, CLIENT_RECONNECT and CLIENT_FORGET notifications (e.g. with the LMSCallbackServer) and update the pool accordingly.

Then, when you get a player object you take it from the pool. When you get a new player, you can check it against the pool one and, if it's equivalent, take the pool one. That way you should be getting the same player object.

I'm just speculating here because I don't immediately see the issue but, you're right, this is something I'm currently expecting the application to handle.

Happy to help if I can.

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

No branches or pull requests

2 participants