Ignore = inaudible ... and invisible?

[JWPlatt]

I had instances of this happening to me on MOULa, which I managed to fix with the PtWasLocallyNotified call and another check, similar to the one you added. I think that what is *probably* happening in this case is that MOSS is sending the directed game message *back* to the sender. In that case, the PtWasLocallyNotified check would fail because only messages that *don't* come through the net client manager will pass that check. There's a flag in the message that gets set and unset. I am unsure if this happens on MOULa or not. Some more testing and code snooping will be required.

Have you tried a'moaca's Wireshark plugin? Please definitely let us know if you find a difference in behavior between MOULa and MOSS. Thanks.

[Tsar Hoikas]

Wireshark is a tool I try to avoid at all costs--I had enough of it when I was teaching myself the UU protocol . In my experience, it's a lot easier to set breakpoints in the debugger and figure out what's going on.

[Christian Walther]

(partial repost from IRC for posterity) I have examined the message-to-self that’s hiding the local avatar when an ignored one links in. It does not come from the server, but directly from where it’s sent inside the client. It looks like it’s inheriting the kNetNonLocal flag from the avatar page message that triggers it, which causes it not to be recognized as WasLocallyNotified. I don’t know enough about messages and cascading to tell whether that’s supposed to happen.

The following is probably a slightly better fix for that than what I posted earlier – it appears to work but I have no idea if I’m breaking anything else by it.

Code: Select all

diff --git a/Python/xKI.py b/Python/xKI.py
index 6e9448a..076d927 100644
--- a/Python/xKI.py
+++ b/Python/xKI.py
@@ -1178,7 +1178,6 @@ class xKI(ptModifier):
     
     def ISendIgnoreNotify(self, clientID, ignore):
         notify = ptNotify(self.key)
-        notify.addReceiver(self.key)
         notify.addNetReceiver(clientID)
         notify.netPropagate(True)
         notify.netForce(True)
diff --git a/Sources/Plasma/FeatureLib/pfPython/pyNotify.cpp b/Sources/Plasma/FeatureLib/pfPython/pyNotify.cpp
index 973979d..7c833a6 100644
--- a/Sources/Plasma/FeatureLib/pfPython/pyNotify.cpp
+++ b/Sources/Plasma/FeatureLib/pfPython/pyNotify.cpp
@@ -197,7 +197,7 @@ void pyNotify::AddResponderState(Int32 state)
 
 void pyNotify::Send()
 {
-    if (!fReceivers.Count())        // Notify msgs must have receivers, can't be bcast by type
+    if (!fReceivers.Count() && !fNetReceivers.size())        // Notify msgs must have receivers, can't be bcast by type
         return;
 
     // create new notify message to do the actual send with

Was there a reason for adding self.key as a receiver, other than to get past the check that I’m extending here – do plNotifyMsgs need a local receiver? If not, then extending the check as part of adding support for net receivers seems to make sense to me.

Another possibility would be to clear the kLocalPropagate flag on the message, but there’s currently no Python API for that.

Edit: Never mind, it actually doesn’t work. And it’s completely obvious why, too: of course the remote receiver needs to know to dispatch the message to the KI too. Doh!

[D'Lanor]

Instead of...

Code: Select all

        if not PtWasLocallyNotified(self.key):

try using this instead:

Code: Select all

        if PtFindAvatar(events) != PtGetLocalAvatar():

Back in October when I was discussing the Minkata sparkly fix with Chogon I asked him why the Python coders had commented out the PtWasLocallyNotified() line in xCalendarStar. This was his reply:

PtWasLocallyNotified() checks to see if the notify message was propagated or locally generated. As notifications got more complex in Plasma, PtWasLocallyNotified() became less reliable in certain situations. But checking if the local avatar was the one that triggered the notification is more reliable.

[Christian Walther]

Thanks D'Lanor, that’s useful to know. Unfortunately, it doesn’t work with the PtEventType.kVariable event used here – these don’t contain avatar information.

However, it amounts to the same thing as the if kiNum == PtGetLocalClientID() I originally suggested – maybe that’s the way to go after all.

PtFindAvatar() could be extended to support kVariable events though if we wouldn’t use ptNotify.addVarNumber() with a dummy number and encode the KI number in the variable name, but ptNotify.addVarKey() with the avatar key. (Assuming there is a way of determining whether a given ptKey points at an avatar – the docstring for ptKey.getSceneObject() sounds a bit scary, but from a cursory look at the code key.getSceneObject().isAvatar() should be safe even if it isn’t a SceneObject.)

[Tsar Hoikas]

I would be more inclined to fix the bug where messages originating from yourself appear to be net propagated because it makes no sense and results in hacks like PtFindAvatar(events) == PtGetLocalAvatar(). While that solution is indeed creative and valid, I would prefer to go after the root cause . Thanks for the suggestion though--I honestly hadn't thought of it even though it's just a matter of adding an activate event to the notify.

I guess my fix will mean some careful testing and review will be needed. Le sigh--I had hoped to avoid a long review process.

[Tsar Hoikas]

Update:

I have addressed all of the bugs you guys have pointed out here. I did some rebasing, so beware . If there are no further objections or such, I'll submit a pull request to the H-uru repo and see about getting it merged into OU as well.

[Christian Walther]

Cool! I’ll have a look over the weekend.

I wonder why GitHub didn’t notify me of this – I’m supposed to be watching you!

Just one quick note before I start testing and commenting on individual commits: The “Send mutual show/hide notifications” revision got absorbed into the “Implement stubs for ptNotify's new APIs” revision, without a mention in the commit message, in the rebasing. Was that intentional?

[Tsar Hoikas]

Just one quick note before I start testing and commenting on individual commits: The “Send mutual show/hide notifications” revision got absorbed into the “Implement stubs for ptNotify's new APIs” revision, without a mention in the commit message, in the rebasing. Was that intentional?

Ah, no it wasn't actually. D'oh!

EDIT: And fixed that >.>

[Christian Walther]

Things are looking good! I did find a couple of problems, but I think I have solved them all.

Your LOS test change does not work as intended for me. It makes all avatars unclickable, not just those with disabled/suppressed physics. Here’s a version that works: 0e580ef. Please review. A comment that may be relevant: plSceneObject.cpp:653.

You forgot to disable clickability when receiving a mutual-ignore notify – fixed in f3ab5a4.

New /ignore Command:
[…] Players that you have ignored will be able to see your text chat (regardless of whether or not you are shouting) and hear your voice chat if they are in range. You cannot see text chat from a player you have ignored nor can you hear their voice chat.

The boldened part is not true in my experience. I think it should be true, so I changed it: 3094cbd. Review appreciated, as I don’t really know what I’m doing here. I tested it with both distance-based (normal) and forced (egg-room) listen lists.

Besides, I have some small additions that I think improve usability of the /shun command:

• Let the output of the command describe more clearly what the command does: d07464d.
• In addition to toggling (/shun), allow the state to be queried and set directly using /shun ?, /shun on, /shun off: 9fb5853. I found that helpful when I forgot what the current state was, but didn’t necessarily want to toggle it.

Let me know what you think.