[Bug]: !rename not working after the first

[jauggy]

Describe the Bug

Recently there has been an updated to SPADS plugins allowing non bossed people to rename a lobby. Try these commands when alone in lobby.

!rename test
!rename test2

Second rename will fail

Reproduce the bug

Ensure you have the latest files from: https://github.com/beyond-all-reason/spads_config_bar/pull/134/files in your local copy of SPADS.

Launch Chobby

!rename test
!rename test2

Second one will fail. Try bossing yourself

!boss jauggy
$rename test
$rename test2

I believe it will now pass

Screenshots

No response

Additional context

I believe that there is a check inside consul_commands.ex When you use $rename as boss it will enter here

      senderid != lobby.founder_id ->
        # We have to do this so we don't block the get_state call from the LobbyServer
        # when it tries to query the rating values
        spawn(fn ->
          :timer.sleep(500)
          Battle.rename_lobby(state.lobby_id, new_name, senderid)
        end)

However, when SPADS tells teiserver to rename that part is not true and instead it goes here:

      lobby.player_rename ->
        state

      true ->
        Battle.rename_lobby(state.lobby_id, new_name, nil)

        state

The first time it will go to the "true" part and work. The next time it will go to the lobby.player_rename part and do nothing.

Need to understand why Teifion put a timer sleep when renaming the lobby when you do it via $rename

[jauggy]

It seems like Teifion added code to prevent spads from updating the name of a lobby after it has be renamed by a player. No idea why it's there. Maybe spads renames lobbies when it's reset?

[DeviousNull]

Well, the only places I can see where SPADS would rename the lobby are 1) the voting PR I wrote, and 2) in this barmanager.py function , specifically here. So, nothing that we would really want to prevent there...

Going back in history, it looks like the "lobby.player_rename" check was added in 3f7a010. But still, I don't see why that would ever be necessary; we control our SPADS plugins (so if we don't want them renaming, we should just patch the plugins), and I can't imagine any reason that SPADS itself would even know about the "$rename" command.

As a sanity check, I played around a bit with SPADS in my dev environment, but I can't get it to call the $rename code in Teiserver without going through one of the two codepaths linked above. !reloadConf and !restart looked like the most likely candidates, but neither of them go through $rename (even though !restart does cause the name to revert back to the default, it doesn't do so through that def).

I think we should just remove the "lobby.player_rename" case for that cond do. If there was a reason for it before, it doesn't look like there is one now.

[DeviousNull]

Maybe it was the case before that a SPADS plugin was responsible for setting the lobby's initial name, and so reloading the plugin also triggered a rename? I'll try to dig through the plugin's history to see if that was true.

[DeviousNull]

Ahh, nevermind my last comments -- it turns out it is that Tachyon function that is the reason for the check to exist. It tries to set the lobby's name when a bunch of different player-modifiable configs change. OK, time to think on how to best handle that...

[jauggy]

Can you explain what function you are talking about? What function/code is this?

[DeviousNull]

Its this line, in barmanager.py : https://github.com/beyond-all-reason/spads_config_bar/blob/9dd4ebebbe3f0b816406da291063a4fe4635c024/var/plugins/barmanager.py#L237

While it is uncommented, the plugin requests a new name for the lobby when certain settings change (easiest to test is the lobby gaining/losing a boss). That's the reason for the "lobby.player_rename" cond on Teiserver's side; without that cond, the plugin would overwrite a custom-set name with an auto-generated one, in response to config changes.

I think we need to split the rename command into two, one command to set the plugin-requested name (restricted to founder) and one to set a custom (player-requested) name. So, basically this is the TODO list I'm considering:

1. Teiserver: Add $set-autoname plugin requested names, which respects lobby.player_rename (current behavior of $rename)
2. barmanager: Change sendTachyonBattleTitle() so it sends $set-autorename instead of $rename
3. Wait for all hosts to get the new code (graceful migration)
4. Teiserver: Update $rename to remove the lobby.player_rename cond

[DeviousNull]

Alternative solution: Move more of the rename logic out of Teiserver and into barmanager.py, so that the plugin is aware of whether a custom name is currently set (and can avoid overwriting it).

This would involve removing "$rename" and writing a new handler for "!rename"... but I think I'd prefer keeping the name logic on Teiserver's side, all else being equal. Easier to update restrictions or blacklist words that way.

[jauggy]

The def sendTachyonBattleTitle(): doesn't even work properly. Tried in prod hosting a room and changing presets and the name didn't update appropriately. I honestly think the request to change name in that function could even be removed.

I think you should remove the rename in that sendTachyonBattleTitleand and maybe add a TODO to have it send up a different way (it's not even used properly now) then remove lobby.player_rename cond in teiserver

[DeviousNull]

SPADS PR done, will work on a Teiserver PR tomorrow

[jauggy]

@L-e-x-o-n this is not fixed can you reopen? @DeviousNull still needs to make changes. Read here: https://github.com/beyond-all-reason/teiserver/pull/374

This PR specifically does not update the $rename command, as that would impact player-set lobby names for any SPADS instances that still use the old BarManager code. Once the new plugin code is running on all servers, I can submit a very short PR to update $rename and fix #371

Basically the $set-config-teaser command now needs to be used by SPADS (change required). Then teiserver needs to change so that the $rename command can now overwrite player-made lobby names. Currently teiserver doesn't allow SPADS $rename to overwrite a player-made lobby name.

$set-config-teaser allows SPADS to make autogenerated names. But auto generated names will never overwrite player-made lobby names.

However if a player calls a !rename vote, then SPADS will use $rename to teiserver. The goal is that $set-config-teaser will never overwrite player-made lobby names, but !rename will.

[L-e-x-o-n]

Ah it got auto closed, sorry.