Some of the muting functionality and UX is broken

mt-mods / beerchat

beerchat mod for minetest

GNU Lesser General Public License v3.0

9 stars 7 forks source link

Some of the muting functionality and UX is broken #115

Closed S-S-X closed 10 months ago

S-S-X commented 10 months ago

Q: Request for a test: is /mute and /unmute actually broken?

A: test with an alt let a $20 whisper through... and the unmute part didn't work. "was not muted", but that muting remains (but does not affect at least local whisper). also /unmute anything-that-is-not-logged-on results in "Unumuted player anything-that-is-not-logged-on." message which seems rather bogus.

Q: Did muting still work for pm and channel messages? A: yes muting worked for channel and pm, but unmuting didn't work when online. Able to introspect resulting state from where ever it is stored?

[x] whispers should get muted.
[x] unmuting should always work, no matter if target exists or not.
[x] unmuting should result in a better reply that indicates if command actually did or didn't do something.
[x] muting players that do not exist should work (mute bridge users).

Athozus commented 10 months ago

Related to mt-mods/mail#128, when syncing the mute list it is giving all the existing players.

S-S-X commented 10 months ago

Wrote few simple test for this, and a lot of it is failing. Seem /mute is pretty messed up currently. It is handling

before_send_pm
before_check_muted (no idea why this is even included, before_send_pm would prob work for every use case)
on_send_on_channel

I guess before_check_muted can stay anyway, might not be a good idea to change it just yet.

Pretty sure I just simply did not really think it through when adding events but just included anything that seemed somewhat appropriate. Back then /mute was part of the core functionality and very tightly integrated with other stuff, isn't like that anymore.

Athozus commented 10 months ago

Should I disable sync function for the moment (transfer works fine) on mail pull request ?

S-S-X commented 10 months ago

Should I disable sync function for the moment (transfer works fine) on mail pull request ?

Actually I don't remember what it was supposed to do... if it isn't needed then probably better disable.

Athozus commented 10 months ago

sync get the data from beerchat and adapt it to mail (called in get_setting), transfer does the contrary (called in set_setting)

S-S-X commented 10 months ago

unmuting should always work, no matter if target exists or not.

Not sure if this got fixed or not. Probably but there's no specific tests for it. Can be tested in prod and issue closed if all is good.

S-S-X commented 10 months ago

I think I've heard last part was also confirmed to work as it should so closing this.