The client list has a cache of the buttons that can be clicked. This is invalidated when the client's connection is closed. However, since 4f6d75f the actual deleting of the NetworkClientInfo is deferred.
This meant that the rebuilding of the cache happened when the client info still existed, and then the UI would be drawn with an out-of-date cache. When the client was not a spectator, this would make the button for the spectator "group" in the UI have functionality related to the client of the previous row. Running any of those commands will cause a crash.
Description
Just do the invalidating in the destructor after the client info has been removed.
Limitations
Tangentially related, the UI at the server will also show clients that have not fully joined yet. This could in theory lead to similar issues. However, solving those cases are far less trivial; the server simply allocated the client info too early for most uses of the client info, but allocating it later will cause duplicate work.
With master this issue is a lot smaller as the client info is allocated after authorizing to the game, and there is no password authorization for companies any more. With many clients waiting to join or slow joiners this could in theory still be a problem.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.
The bug fix is important enough to be backported? (label: 'backport requested')
This PR touches english.txt or translations? Check the guidelines
This PR affects the save game format? (label 'savegame upgrade')
This PR affects the GS/AI API? (label 'needs review: Script API')
ai_changelog.hpp, game_changelog.hpp need updating.
The compatibility wrappers (compat_*.nut) need updating.
This PR affects the NewGRF API? (label 'needs review: NewGRF')
Motivation / Problem
Fixes #12655.
The client list has a cache of the buttons that can be clicked. This is invalidated when the client's connection is closed. However, since 4f6d75f the actual deleting of the
NetworkClientInfois deferred.This meant that the rebuilding of the cache happened when the client info still existed, and then the UI would be drawn with an out-of-date cache. When the client was not a spectator, this would make the button for the spectator "group" in the UI have functionality related to the client of the previous row. Running any of those commands will cause a crash.
Description
Just do the invalidating in the destructor after the client info has been removed.
Limitations
Tangentially related, the UI at the server will also show clients that have not fully joined yet. This could in theory lead to similar issues. However, solving those cases are far less trivial; the server simply allocated the client info too early for most uses of the client info, but allocating it later will cause duplicate work.
With master this issue is a lot smaller as the client info is allocated after authorizing to the game, and there is no password authorization for companies any more. With many clients waiting to join or slow joiners this could in theory still be a problem.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.