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

Feature: News/advice setting to warn if no depot order in vehicle schedule #8966

Closed
wants to merge 1 commit into from

Conversation

2TallTyler
Copy link
Member

Motivation / Problem

One of the biggest frustrations players report with breakdowns is trains getting lost when they hit their automatic service interval and wander off to find a depot.

This is easily fixed by giving the train an explicit service order, which disables automatic servicing. But this is not obvious to players, who assume breakdowns are "broken" and disable them.

Description

This is an upstreamed patch from @JGRennison, with some minor changes.

When breakdowns are enabled and a vehicle doesn't have a depot order in its schedule, a warning message is generated similar to when a vehicle's income is negative, it has duplicate orders, or it has incomplete orders. This nudges players toward explicitly giving depot orders, which will prevent their trains from getting lost trying to auto-service.

Aircraft are not included in these checks, because when they hit their service interval they continue their flight and service at their destination airport's hanger before heading to the terminal to load/unload. Explicit depot orders are not necessary. Helicopters are serviced when they land at the 1-tile skyscraper heliports and at oil rigs, even though these don't have hangers.

Obviously, this is not the only way to play with breakdowns. This warning message can be turned off in Settings > News / Advisors, like the other vehicle checks. It is enabled by default to aid new players, following the precedent we've set with #8463.

Limitations

This feature is intentionally active only when breakdowns are enabled. However, when autoreplace* is used, vehicles needing replacement, which don't have a depot order, will still wander off to find a depot when they hit their automatic service interval. I think this is less of a problem and forcing this feature upon players who disable breakdowns might be frustrating. Thoughts?

*Autoreplace upgrades vehicles to another type, as opposed to autorenew which simply buys a new copy of the same vehicle.

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')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@perezdidac
Copy link
Contributor

I am a fairly new dev but based on my limited experience I can anticipate a push against this from other developers, mainly for two reasons:

  1. adding a new game settings to the game
  2. this, by default, shows the warning, which will effectively teach new players to add a depot as part of the orders

2 is key here, as I am not sure if that's how the majority of people play.

On the other hand, I have to say that we need to solve the problem of a train needing service at a time X and ending up deviating into a line that it will struggle to come back from. A while ago I explored the option in Pathfinder of optimizing those routes so the train can keep going to the destinations until it finds a depot but without having to deviate crazily. I am happy to work together in this if you are in the mood 👯

@2TallTyler
Copy link
Member Author

  1. I'm all about minimizing settings, but sometimes they are necessary.
  2. Teaching new players to add a depot is the point of this PR, as explained in the description above. This is a precedent we set in Change: Default settings improved for new players #8463, whereby default settings are tuned for new players even if they are suboptimal for more experienced players (a good example is Show Path Reservations). It is easy for experienced players to tune such settings to their own liking. But personally, I would leave this setting on for myself, so I don't think it only has ramifications for new players. (Plus, it is inactive when breakdowns are off, so I suspect most longtime players would never see it).

Your proposed pathfinder changes would be useful, but I suspect the variability of where players place depots will make this more difficult than it sounds.

@perezdidac
Copy link
Contributor

2. inactive when breakdowns are off,

Hey! Sounds great :) I like that the feature is inactive when breakdowns are off.

To be honest, I am happy with this PR, I just wanted to raise the fact that some current players will get surprised by this new message. But I think you have pretty much clarified in the PR description. Are all 'official' wikis and tutorials reflecting this?

@auge8472
Copy link
Contributor

I for myself set depot orders to every order list so I would be not affected. But there are other players who intentionally does not set such orders and I suspect they would not like to get tons of warnings. 🤔

@2TallTyler
Copy link
Member Author

Thus the setting to disable this feature. 😀

@LordAro LordAro added the candidate: needs considering This Pull Request needs more opinions label Apr 11, 2021
@auge8472
Copy link
Contributor

Thus the setting to disable this feature. grinning

This IMHO is the wrong, twisted logic because it "breaks" existing games and game play styles. Default should be to have to deliberately enable this setting (again: IMHO).

@2TallTyler
Copy link
Member Author

This is simply a client news setting which is easy to disable, and leave it that way forever. It does not and cannot "break" anything.

We discussed breakdowns in #8393 and the consensus was to fix them, not disable them by default. This PR is an attempt to help fix them. As such, it is default-on to aid new players in playing with breakdowns when they first pick up the game. This is the attitude and precedent we've set in #8463.

@auge8472
Copy link
Contributor

This is simply a client news setting which is easy to disable, and leave it that way forever. It does not and cannot "break" anything.

Yes, "break" is a strong word for what happens and because of that I set it into quotes (here and in my previous comment). In the end, as a player, I would be annoyed to get a possibly big amount of warnings in existing games (or at leasrt in the first game I restart until I change the setting (I have to know about)). This is my opinion even I would not be affected by this change (as stated in my first comment).

@nielsmh
Copy link
Contributor

nielsmh commented Apr 11, 2021

This would yell at you even if you were using a classic forced servicing track layout for your trains.

@2TallTyler
Copy link
Member Author

This would yell at you even if you were using a classic forced servicing track layout for your trains.

Yes, it would. My thinking is that a player who knows how to build a forced servicing layout would be able to turn off the setting. The goal is to help new players who don't know how to handle autoservicing with the breakdowns which are on by default.

src/order_cmd.cpp Outdated Show resolved Hide resolved
@rubidium42
Copy link
Contributor

I would be all for enabling something that helps new players in the game, even if it might show warnings to more experienced users in particular use cases. I can also agree with the notion that people must know they can disable this warning, so a maybe more general solution for that should be devised.

I can think about some manner to dismiss a whole class of warning messages from the popped up news message, though that is far outside the scope of this PR. Then the advanced players can disable them with a single click and the new players get some of the help they need. It would even give the advanced players some notion that a new check has been implemented and they can actively dismiss it if they are not pleased with it, instead of not knowing it exists when it would have helped greatly in their game.

I could even imagine that some of those message types could actually be disabled on a game-by-game basis. If you were to play on a server with a forced servicing layout, then for the first warning you say something like "disable for this game" and it will not complain about it for you and all the other players in your company. When you join/load another game, it would then default back to your normal configuration of the news setting. However, that would be even outside of the scope for the previous proposal, though it might help in not flat out disabling all kinds of useful checks or having new players join the game that then get spammed by depot warnings.

@2TallTyler
Copy link
Member Author

I agree that a better way to manage the News/Advisor settings might be helpful. I've seen other games do this with a "Show Tips" checkbox when starting a new game, and sometimes a "Hide Tips" button on the tips themselves. Out of scope for this PR of course, but perhaps a helpful thing to keep in mind for those worried about adding new warnings to bother players.

@2TallTyler 2TallTyler force-pushed the depot_order_warning branch 2 times, most recently from 87a212d to 703d80e Compare October 20, 2021 14:49
@2TallTyler 2TallTyler added the size: trivial This Pull Request is trivial label Nov 17, 2022
@2TallTyler
Copy link
Member Author

There doesn't seem to be an appetite for this feature, at least not for the amount of complaints it would generate from players.

@2TallTyler 2TallTyler closed this Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: needs considering This Pull Request needs more opinions size: trivial This Pull Request is trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants