New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change: Return 'New company/Spectate' option to company toolbar menu #9532
Conversation
Yes, it didn't make it in the PR description. That is my bad. We talked it over outside of GitHub, and as it goes, that doesn't always make it into the PR description. It was already really lengthy, and multiple people indicated it was kinda obvious we remove those entries from the menu too. So why are they removed? Well, several reasons honestly (these are mine, others might have more to pitch in): First, them being there was always a hack. Back in the day, I (and I can only blame myself here) was very lazy, and couldn't find a way to add it to the network GUI. So the next best thing was used, with the idea it was "temporary". 14 years later .. yeah .. that didn't happen. In addition to that, it also turns out it was really difficult to find for people. With the introduction of the new Online Players GUI it makes much more sense to put it there: all multiplayer related things close together. This is also why it was ruled "obvious" to remove it from the menu: having the same functionality twice is a bit silly, and in the GUI it makes much more sense. Finally removes the weird quirk in that menu. As for "convenient shortcut", I think that holds true for nearly any functionality you personally use or have gotten used to. Personally, I would like to have scroll-to-HQ in the menu, but alas, we cannot endless add things to the menu. So on the convenient argument, I am not sold. As alternative, having fully customizable menus might be a fun (and goofy) idea. I do acknowledge that for existing users it can be a bit like: huh? Where did that go? But given we did a lot to indicate there is a new GUI with this functionality, I think most players will not miss it after a bit of time. As after all, how often do players really change to spectator? In other words: I think our GUI should tailor to the 90%. As such, I think this PR should be closed. It doesn't offer any useful functionality for most of our players, and it feels like it is only scratching an itch for a select few. I can be convinced otherwise, of course, but as it stands, I think it is obvious that #9067 removed this from the menu. |
Feels ok to me, there is a company list and new company / spectate much like in "Online players", even better imo. If anything it's "Online players" that's out of place there since it's a list of companies linked from the list of companies. And, speaking of confusion, depending on "who you are" bald guy button opens either "online players" window or company window, much more confusing than replacing options if you ask me :p
Not much of an issue now that it's not the only way to do it.
I'm not suggestting removing it from "online players" (even though it could use a bit of redesign there imo). And there are plenty of examples of other convenience "shortcuts" in the game. Including company window join button that duplicates the functions of the new GUI.
How often do players really use anything in "online players"? I'd argue that new company/spectate are by far the most used functions there. Unless more players suddenly discover they can talk to companies/clients and start doing that. |
"New Company" should have a confirmation window because before the change it was far too easy to accidentally create a new company. |
Also interesting, turns out people use "Spectator" to mark themselves as "away". So in a use-case:
In short, this is not the intended use of spectators ofc, but it is a neat way of dealing with it. It is also most likely specific to certain servers, as the common player would not know about this, neither follow this rule. This insight helped me a bit, as I couldn't understand why people would want to go to spectators if they started to play the game. But if you created your own workflow on top of it, it makes sense of course :) Which circles back to this PR, as that means for "Spectate" we have more than "it is convenient" argument (on which I am still not sold). So I browsed a bit through past complaints about those buttons. Our menu is mostly "safe". This is to say: if you click any buttons, it either asks you to confirm, shows you a GUI, changes your mouse-input, or is a toggle. This means if you click somewhere by accident, the road to recovery is always clear. There are three exceptions to this in pre-12.0:
The latter hasn't resulted in any complaints (yet), but the other two have. @EmperorJake also mentions this rightfully, that especially "New company" has been an issue. And this is what I mentioned earlier: as those two buttons swap places, the UX becomes very unclear. A user isn't shown how to get back to their original company after pressing "Spectate". Pressing the same location in the menu again DOES NOT bring you back to the earlier state. Many users reported this over time as confusing, got lost, irritated, etc. In other words: we really shouldn't revert back to this state, as it clearly wasn't a good place to be in. So this PR as-is, we really shouldn't do. The "New Company" specifically, is also the reason we removed the lobby window: people clicking it by accident, not really knowing what it means. So in my opinion, the only place to start a new company should be in the Online Players window. As it is more likely the user will realise they wanted to join a company, and do that instead. The "Spectate" button is the more interesting one. Especially as it is used as an "I am away" button. Having that conveniently available, becomes an argument worth talking about. Someone is at your door, you just quickly want to mark yourself as away. Opening the right GUI to find the right button, yes, that is an argument I can understand. Which creates three questions in my head:
Tnx a lot to DIscord where Azusa and Wahazar were very helpful in explaining their use-case. I really appreciate you guys, and that was most helpful :) |
First of all, while afk is one use-case you seem to entirely forget what, I suppose, is the intended use of spectator mode - spectating. And while spectating you main want to
Then there is the case when your company is doing poorly and you want a new one, that currently requires spectating first. Also, there is a twist that in this case and 3-4 of the list you may want to reset your current company which involves typing And that's just something off the top of my head, there may be more use-cases. So while I agree that there may be better UI for these cases I think that until it's developed returning "new company/spectate" menu option is the right step as it should've never been removed in the first place. I only missed that during #9067 development because I never expected that and it was never mentioned anywhere I looked.
As someone (and afaik the only one) who brought up that reason in the first place, let me tell you - "new company" in the menu was never the problem. One that was used as "join server" button was :p P.S. I think "away" state should be automatic. |
I am happy you speak for everyone. |
I think it's important that nobody develops an ulcer over OpenTTD. It's just cartoon trains. |
Motivation / Problem
Somehow this option was removed in #9067 and since I couldn't find any reason for that change and I'm adding it back in cmclient anyway I thought I may as well PR it to vanilla.
It's a convenient shortcut that was already there and doesn't have much to do with server lobby merging with the client list.
Description
Returns "New company/Spectate" option to the company menu ("bald guy" button) in the main toolbar.
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 affects the save game format? (label 'savegame upgrade')This PR affects the GS/AI API? (label 'needs review: Script API')This PR affects the NewGRF API? (label 'needs review: NewGRF')