Cleanup: remove max_nick_length #109
Conversation
This setting is no longer needed as the information can be gained via the listener at runtime. This changes things to use go-ircevent's ISUPPORT support instead of defining it in the config. Closes qaisjp#106.
llmII
force-pushed
the
deprecate-max-nick-len
branch
from
e58253f
to
d74cf45
Compare
| // Maximum length of user nicks aloud | ||
| viper.SetDefault("max_nick_length", ircnick.MAXLENGTH) | ||
| maxNickLength := viper.GetInt("max_nick_length") |
There was a problem hiding this comment.
lets tell the user that it's not detected at runtime and they should remove this field from their config
There was a problem hiding this comment.
I'll add it like we did for nickserv_identify (or you can if you want, I won't be able to get back to this till tomorrow, life events).
There was a problem hiding this comment.
Done. In this one I merely inform the user, but don't actually exit the program. Let me know if we should tell the program to exit in this case.
| @@ -302,7 +302,7 @@ func (m *IRCManager) generateNickname(discord DiscordUser) string { | |||
| suffix := m.bridge.Config.Suffix | |||
| newNick := nick + suffix | |||
|
|
|||
| useFallback := len(newNick) > m.bridge.Config.MaxNickLength || m.bridge.ircListener.DoesUserExist(newNick) | |||
| useFallback := len(newNick) > int(m.bridge.ircListener.NickLength()) || m.bridge.ircListener.DoesUserExist(newNick) | |||
There was a problem hiding this comment.
this acquires a lock, so might not run great when connecting 300 people. but lets leave it in here for now unless you can think of an easy better solution.
There was a problem hiding this comment.
Good catch, might be best to handle figuring out NickLength once per OnWelcome event in the listener and set it as a field on the Listener. However, probably need to also do an on disconnect and quit ordeal so as to clear it to a "hold up, we're not connected yet) value. Probably is overkill that last part because if a server changes this between the time of a quick disconnect and reconnect there's probably going to be other issues anyway...
Same as above applies... I'll try to get this implemented like I noted above tomorrow or if you want to you can take over for it, life events are gonna keep me away from it for the rest of today :/
There was a problem hiding this comment.
That's okay, life is more important than a random internet irc bridge :) Take care of yourself!
There was a problem hiding this comment.
IIRC IRCd's send 376 (end of motd), and 422 (no motd found). Ideally the client should listen for these events and then process their prejoin commands and access ISUPPORT (which was sent earlier in 005). Should we go forward this direction instead of processing at 001?
Even if we do, I need to lock the HandleUser function out from running until 376/422 are handled and a NickLength is stored on the Listener. So to achieve a "don't lock per puppet creation" we sort of need the Listener to have a NickLength() call that with a sentinel variable that way we only lock once for the entire ordeal of getting the NickLength.
How expensive is a lock? would it actually make it impossible for 300 users to connect in the same second. I think I'm going to do the above but if you figure it's too complex or have a better idea I could go for that instead.
There was a problem hiding this comment.
I think this should be committed, and a separate PR to fix things to use 376/422 should be created (which would include locking). I'm not trying to split things up too much but that's possibly a change that would need far deeper review than this change here.
It seems to work, but very good call on wanting to test this! |
We now inform users they should remove this field from their config as it is no longer necessary.
|
Did you manage to test whether ISUPPORT is fetched from the listener before connecting the puppets? |
This setting is no longer needed as the information can be gained via the listener at runtime. This changes things to use go-ircevent's ISUPPORT support instead of defining it in the config.
Closes #106.
This one will need some testing, creating a draft. Need to review and see if puppets are only created after the listener has processed OnWelcome or if it is possible they exist before in which case getting the value from ISUPPORT isn't possible since the listener doesn't exist yet...