Added handling for relationship presence updates #494
Conversation
| ? Game.GameType.DEFAULT | ||
| : Game.GameType.fromKey(Integer.parseInt(content.getJSONObject("game").get("type").toString())); | ||
| : Game.GameType.fromKey(Integer.parseInt(game.get("type").toString())); |
There was a problem hiding this comment.
Wouldn't game.getInt("type") be the same here?
There was a problem hiding this comment.
In the past we've encountered numerous issues with game objects due to them not being checked by the API, I didn't touch that logic at all here.
| if (friend == null) | ||
| { | ||
| if (status != OnlineStatus.OFFLINE) | ||
| return null; |
There was a problem hiding this comment.
What is the point of this?
There was a problem hiding this comment.
ugh, looks like I forgot to remove this after I decided not to cache these presence updates
| if (content.has("guild_id")) | ||
| { | ||
| GuildImpl guild = (GuildImpl) api.getGuildById(content.getLong("guild_id")); | ||
| if (guild != null) |
There was a problem hiding this comment.
The removal of the //cache in relationship stuff isn't correct here.
We need to be caching this presence for a very soon Group add, similar to how we are doing for the Guild presence caching.
There was a problem hiding this comment.
I'm not convinced we should be caching relationship presence updates, groups don't send these.
|
@DV8FromTheWorld can I get an update on your view on this PR? |
| api.getEventManager().handle( | ||
| new UserGameUpdateEvent( | ||
| api, responseNumber, | ||
| user, null, oldGame)); |
There was a problem hiding this comment.
We should be firing FriendXEvents, not UserXEvents I think.
In that same vain though, we probably should rename the User events to Member events?
There was a problem hiding this comment.
Or, at a minimum, take a Friend object to these User events so that you can call getFriend like you can call getMember.
There was a problem hiding this comment.
That is already supported in message-rw changes: https://github.com/DV8FromTheWorld/JDA/pull/453/files#diff-b7dbbd6a832a00b7f5e6173633fefff6
MinnDevelopment
force-pushed
the
feature/presence-updates
branch
from
beb11d5
to
3356d7e
Compare
…ebugging capabilities in the future
…date # Conflicts: # src/main/java/net/dv8tion/jda/core/requests/WebSocketClient.java
| @@ -208,11 +208,9 @@ protected Long handleInternally(JSONObject content) | |||
|
|
|||
| //If the OnlineStatus is OFFLINE, ignore the event and return. | |||
| OnlineStatus status = OnlineStatus.fromKey(content.getString("status")); | |||
| if (status == OnlineStatus.OFFLINE) | |||
There was a problem hiding this comment.
We lost the ability to return in an OFFLINE non-guild situation.
Is that correct handling?
There was a problem hiding this comment.
Can you elaborate? Not sure what you mean
There was a problem hiding this comment.
Nevermind. I see what has changed here. The fast escape logic is no longer needed.
| @@ -1319,8 +1321,11 @@ private void setupHandlers() | |||
| handlers.put("VOICE_STATE_UPDATE", new VoiceStateUpdateHandler(api)); | |||
|
|
|||
| // Unused events | |||
| handlers.put("CHANNEL_PINS_UPDATE", new SocketHandler.NOPHandler(api)); | |||
| handlers.put("WEBHOOKS_UPDATE", new SocketHandler.NOPHandler(api)); | |||
| final SocketHandler.NOPHandler nopHandler = new SocketHandler.NOPHandler(api); | |||
There was a problem hiding this comment.
Technically, since NOPHandler will never be implemented, we should have it be a public static final instance in SocketHandler instead of in-line creating one here.
There was a problem hiding this comment.
The reason is that the SocketHandler constructor expects a JDA instance and I don't want to add confusing situations where that JDA instance isn't present/null
There was a problem hiding this comment.
Okay, I can get behind that.
However, lets create the final variable at the top of the method, not in the middle.
MinnDevelopment
added
the
status: completed
No description provided.