mt-mods / beerchat

beerchat mod for minetest
GNU Lesser General Public License v3.0
9 stars 7 forks source link

Fix /tell override crash when used without recipient or message #113

Closed S-S-X closed 1 year ago

S-S-X commented 1 year ago

Some tests for #112

When done, fix #112

S-S-X commented 1 year ago

Prob fine but not sure how is_muted should actually behave if target is nil, despite what I said before I think I'm leaning towards letting it crash.

Maybe it should throw warning to logs, after all it is something that should not normally happen.

Or maybe after all it shouldn't actually check for invalid input but just let it crash so that actual underlying issue, that might well break other things, wont get hidden.

S-S-X commented 1 year ago

am going to change it and make it clear that target is needed for mute check.

S-S-X commented 1 year ago

Changed to allow original mesecons /tell to handle empty target parameter. And changed pattern to allow calling through checks even if there was no message after recipient. And changed is_muted resilience to assertion to make responsibilities of caller clear and to keep root cause visible.

@BuckarooBanzay what do you think about update? My reasoning for changes is that I think first version was actually very bad because it would hide root cause for issues that could anyway easily emerge elsewhere in callback chain.

BuckarooBanzay commented 1 year ago

@BuckarooBanzay what do you think about update? My reasoning for changes is that I think first version was actually very bad because it would hide root cause for issues that could anyway easily emerge elsewhere in callback chain.

looks good :+1: