Bug with MCCP toggling

Bug #1119884 reported by Vadim Peretokin
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mudlet
Fix Released
Wishlist
Unassigned

Bug Description

If MCCP is turned on, then off, then on again, binary data is output to screen. The first toggle on and off work correctly.

As this is a new MCCP implementation on the server, it was tested in CMUD and tf, both work as expected.

See http://forums.mudlet.org/viewtopic.php?f=5&t=3204&p=15891&hilit=mccp#p15891 for the original report.

Heiko (koehnheiko)
Changed in mudlet:
status: New → Won't Fix
status: Won't Fix → New
importance: Undecided → Wishlist
Revision history for this message
Michael (deathlire) wrote :

Along with a helper of mine we've fixed the issue of turning off and on for MCCP. Works as well as it should even during copyover which is one reason it really needed a fix. Including a patch which is for the src in the new Mudlet-3.0.0-iota.tar.gz and not the latest git development. Should be an easy fix for development versions as well. There were a few more problems not noted with other protocols, but that's for somebody else for now. Thanks -Darksix.

Revision history for this message
Vadim Peretokin (vperetokin) wrote : Re: [Bug 1119884] Re: Bug with MCCP toggling

Awesome, thanks! Appreciated.

On Mon, 13 Mar 2017 5:54 am Michael, <email address hidden> wrote:

> Along with a helper of mine we've fixed the issue of turning off and on
> for MCCP. Works as well as it should even during copyover which is one
> reason it really needed a fix. Including a patch which is for the src
> in the new Mudlet-3.0.0-iota.tar.gz and not the latest git development.
> Should be an easy fix for development versions as well. There were a
> few more problems not noted with other protocols, but that's for
> somebody else for now. Thanks -Darksix.
>
> ** Patch added: "fix for mccp toggle and copyover bug"
>
> https://bugs.launchpad.net/mudlet/+bug/1119884/+attachment/4836695/+files/mudlet-copyover.patch
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1119884
>
> Title:
> Bug with MCCP toggling
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mudlet/+bug/1119884/+subscriptions
>

Michael (deathlire)
Changed in mudlet:
status: New → Confirmed
description: updated
Revision history for this message
Michael (deathlire) wrote :

Let me know how it works for you Vadim. Was tested against MCCP version 2 so far.

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

Would you mind providing us with a way to test this?

If you know of several codebases we could test it with, that'd be even better, otherwise I'll dig around google.

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

PRs https://github.com/Mudlet/Mudlet/pull/423 and https://github.com/Mudlet/Mudlet/pull/422 apply this patch, please let me know if there's anything off.

Changed in mudlet:
milestone: none → 3.0
status: Confirmed → In Progress
Revision history for this message
Michael (deathlire) wrote :

Sorry I had a little mess up on the diff patch, those 2 lines were needed, putting up the new one now. Also you can use darklord.evils.in port 9100 to test it. It really hated the way it used compression. it's a Godwars based mud of mine.

Last fix for patch did not show characters during connect. This one does. Sorry about the confusion.

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

Confirmed fixes test case for darklord.evils.in:9100 (create a character and type 'compress' to toggle MCCP on/off)

Confirmed fixes test case in http://www.mudbytes.net/forum/comment/72091/#c72091

Test case works on coffeemud.net port:2323 as well

IRE games still work fine

PRs https://github.com/Mudlet/Mudlet/pull/424 and https://github.com/Mudlet/Mudlet/pull/425 submitted with the working and QA'd patch.

I think we're kosher. Thanks for the patch Michael!

Revision history for this message
Michael (deathlire) wrote :

No problem, also RockHound's email name is his name backwards, as I had been told. far as words go. Was about to show you that also copyover works and disables, then auto negotiates the client again. And I'm not certain but the original patch may have also worked, but for now we will rely on this until i test further and on more codebases. So far it's worked on all the problematic ones with my version with this patch with all Qt libs being compiled as debug and static when installing them all on Gentoo, well my derivative anyways. Source was patched originally by your diff's to how gentoo handles Lua, all seems well. I'm just sorry I didn't start with the dev branch from git like RockHound did. But over all seems to work for windows and linux just fine. Glad it worked out for both versions and it wasn't too far along to change.

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

No drama! It's in and will make into the 3.0 release, thanks for the contribution on improving Mudlet! Hopefully the process was painless enough for you to try again in the future :)

By the way, Mudlet really likes that config +telnetga option, if you could enable it by default for Mudlet users...

Changed in mudlet:
status: In Progress → Fix Committed
Revision history for this message
Michael (deathlire) wrote :

Thanks, and also Rock found a way to get rid of a ton of bloat for that compression fix. Can get a diff patch up sometime soonish for that, and if you want telnet ga turned to on when it connects to a server? That also could be done, would just have to issue a few other checks of supporting it or not. Would also like to fix *needing to reconnect* to switch the options off where force compression is also at. Sometime in the future so it could be instantaneous like it really should be.

Revision history for this message
Vadim Peretokin (vperetokin) wrote :

Sounds great, improvements always welcome. Let's try a pull request on the development branch in https://github.com/Mudlet/Mudlet this time - much easier workflow than back and forth patches :)

Changed in mudlet:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.