Skip to content
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

Add: Client setting gui.start_spectator #7158

Closed
wants to merge 1 commit into from

Conversation

@SamuXarick
Copy link
Contributor

SamuXarick commented Feb 1, 2019

Activates a spectator slot in single player and initiate games as a spectator

@SamuXarick SamuXarick force-pushed the SamuXarick:start-as-spectator branch from c77450b to 30cd9d6 Feb 1, 2019
@@ -67,8 +67,8 @@ static int32 ClickMoneyCheat(int32 p1, int32 p2)
*/
static int32 ClickChangeCompanyCheat(int32 p1, int32 p2)
{
while ((uint)p1 < Company::GetPoolSize()) {
if (Company::IsValidID((CompanyID)p1)) {
while ((uint)p1 <= COMPANY_SPECTATOR) {

This comment has been minimized.

Copy link
@glx22

glx22 Feb 1, 2019

Contributor

I think that's wrong, you should still check Company::GetPoolSize(), but you can add something to the test

src/company_gui.cpp Outdated Show resolved Hide resolved
src/company_gui.cpp Outdated Show resolved Hide resolved
src/company_gui.cpp Outdated Show resolved Hide resolved
src/network/network.cpp Outdated Show resolved Hide resolved
src/openttd.cpp Outdated Show resolved Hide resolved
} else {
*list->Append() = new DropDownListStringItem(STR_NETWORK_TOOLBAR_LIST_SPECTATOR, CTMN_SPECTATOR, false);
}
}

This comment has been minimized.

Copy link
@glx22

glx22 Feb 1, 2019

Contributor

that's a lot of copy pasting, but maybe no other way

@SamuXarick SamuXarick force-pushed the SamuXarick:start-as-spectator branch 2 times, most recently from af2ed56 to 02f3bda Feb 2, 2019
plane = CWP_MP_C_PWD;
} else {
plane = CWP_MP_C_JOIN;
}

This comment has been minimized.

Copy link
@PeterN

PeterN Feb 2, 2019

Member

Excessive expansion, plane = local ? CWP_MP_C_PWD : CWP_MP_C_JOIN; would be fine, and similar for the block above.

Copy link
Contributor

glx22 left a comment

I think you should not touch economy.cpp and bankruptcy handling

src/company_gui.cpp Outdated Show resolved Hide resolved
src/toolbar_gui.cpp Outdated Show resolved Hide resolved
@SamuXarick SamuXarick force-pushed the SamuXarick:start-as-spectator branch 4 times, most recently from 43ee27c to bb1c94d Feb 3, 2019
@SamuXarick SamuXarick force-pushed the SamuXarick:start-as-spectator branch 4 times, most recently from 5e94b8c to 9ec1d48 Feb 16, 2019
Activates a spectator slot in single player and initiate games as a spectator
@SamuXarick SamuXarick force-pushed the SamuXarick:start-as-spectator branch from 9ec1d48 to 2b95f53 Feb 18, 2019
@TrueBrain

This comment has been minimized.

Copy link
Member

TrueBrain commented Mar 2, 2019

I fail to see the usecase for this behavior. Why would I want to be a spectator in a SinglePlayer game? Feels like an anti-pattern. And given the PR does not argue why it should be added, going to close the PR.

From a short chat on IRC, I understand it is to watch the AI do its thing. I don't see how it is useful to change so many lines of code for a very low usercount that will ever use it that way. So if that is the argument, I am still going to close the PR.

Sorry, not every feature can make it into OpenTTD :) But thank you for your contribution nevertheless!

Feel free to rebuttal. But at always, do this with new arguments. Tnx!

@TrueBrain TrueBrain closed this Mar 2, 2019
@stale

This comment has been minimized.

Copy link

stale bot commented Mar 20, 2019

This pull request has been automatically marked as stale because it has not had any activity in the last month.
Please feel free to give a status update now, ping for review, or re-open when it's ready.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.