Page MenuHomeWildfire Games

XmppClient gloox message subtype specific JS message handlers
Needs ReviewPublic

Authored by elexis on Sep 14 2019, 11:57 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#5348
Summary

As reported in #5348, if one changes the banlist in psi+, there will be a message sent from the xmpp server that contains no body and no subject but an item of the changed username and affiliation (none or outcast dependin on unban or ban).
This message manifests as an empty message box the way gloox parses it currently.
By passing the gloox message subtype enum to JS, the JS side has a better way of distinguishing all message types.

The downside is

  • that the !subject !body tests still need to be performed with the current gloox / glooxwrapper code.
  • that some message combinations are expected never to occur, or reflect incomplete knowledge. If these combinations cant occur, it would be good to remove them in gloox. If it is incomplete knowledge and the message can occur, we will now get a warning and we see it manifested in the code where it could occur, and we have an easy way to insert the new message handler.
  • bit of duplication for private messages (having that check in JS still seems odd to me, it should be set on the server if we dont want users PMing)
Test Plan

Test this patch in specific for:

  • Lobby subject change (only possible in psi+ not pidgin)
  • Kicking people
  • Modifying the banlist in psi+ (not possible on pidgin)
  • Sending an announcement
  • Sending a private message from a moderator account and a regular player account
  • End up reading gloox code and figuring out what's missing there.

Event Timeline

elexis created this revision.Sep 14 2019, 11:57 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/677/display/redirect

elexis added inline comments.Sep 14 2019, 12:01 PM
binaries/data/mods/public/gui/lobby/lobby.js
171

As reported by riversa during the test announcement stage yesterday, the popup boxes should not be displayed again when the user opens the lobby dialog again (only the first time).

(08:35:46 PM) ValihrAnt: is the announcement showing up everytime i look at lobby room intended/
(08:36:24 PM) rivesara: message of theday boday and test annoouncemeny
(08:37:18 PM) rivesara: it is comming again again
(08:38:21 PM) rivesara: elexis everytime i close and open lobby same msg appear again

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/167/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/678/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/679/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/680/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/681/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/745/display/redirect

Stan added a subscriber: Stan.Sep 18 2019, 2:26 PM
Stan added inline comments.
binaries/data/mods/public/gui/lobby/lobby.js
135

"Private messages between users are not supported." ?

138

Capital S, final dot.

159

Missing dot.

source/lobby/XmppClient.cpp
1328

Invalid Message ?

elexis added inline comments.Sep 18 2019, 5:05 PM
binaries/data/mods/public/gui/lobby/lobby.js
135

That doesn't express that it is intentionally not supported, so someone will remove the line and can claim to add support.
This check is shet, because every JS mod can remove it, it should be a room policy, but then moderators and selected individuals cant PM anymore. Must deem offtopic!

source/lobby/XmppClient.cpp
1328

This is not a string to be read by humans but a string identifier for JS (otherwise the strings above would be highly broken)