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

Jump to bottom

Fix #7972: show invalid orders to stations that don't accept your vehicle #8516

Merged
merged 1 commit into from Jan 8, 2021
Merged

Fix #7972: show invalid orders to stations that don't accept your vehicle #8516

merged 1 commit into from Jan 8, 2021

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 7, 2021 •
edited

Fixes #7972

Motivation / Problem

Having invalid orders that are shown as regular orders heavily confuses any user. So we have to do something to make it more .. clear what is going on. See #7972 for more detail.

Description

Before it was shown as a normal order, but the vehicle was skipping
it. This was rather unclear to the user. Now it is red and contains
text with some hints what is going on.

The text is prefixed rather than post-fixed, as we have many
post-fixes already.

For a train it looks like this:
image

For a bus it looks like this:
image

And in a very extreme case it looks like this:
image

Limitations

  • Order GUI already makes for way too long of a lines; this is adding to that problem. But I currently see no other way except to completely overhaul the GUI, which is just ever so slightly out of scope (sarcasm implied ;)).

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')

@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 7, 2021
@LordAro
Copy link
Member

LordAro commented Jan 7, 2021

I think I liked the other option mentioned in the issue, with the "Can't use station" text at the end of the order. Sure, it might be off the end of the order window, but the text is still red, so making it obvious that there's a problem

@TrueBrain
Copy link
Member Author

I think I liked the other option mentioned in the issue, with the "Can't use station" text at the end of the order. Sure, it might be off the end of the order window, but the text is still red, so making it obvious that there's a problem

Problem is a bit how order-strings are created. There are whitespaces at the end of the string (which you normally don't see). As you can see on the other screenshot, it becomes very clear with these kind of orders. And solving that .. boy, that is difficult :D That is why I went for this approach .. do we have any other suggestions by any chance?

Additionally, for trains it becomes "a (b) [c] (d)", which felt really weird.

@TrueBrain
Copy link
Member Author

TrueBrain commented Jan 7, 2021 •
edited

By popular request, not only "can't use station" is red.

image

@TrueBrain
Fix OpenTTD#7972: show invalid orders to stations that don't accept y…
f89cd92 
…our vehicle

Before it was shown as a normal order, but the vehicle was skipping
it. This was rather unclear to the user. Now it is red and contains
text with some hints what is going on.

The text is prefixed rather than post-fixed, as we have many
post-fixes already.
LordAro
LordAro approved these changes Jan 8, 2021
View reviewed changes
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming no issues with PUSH/POP_COLOUR with eints/whatever

@TrueBrain TrueBrain merged commit 9aa39d0 into OpenTTD:master Jan 8, 2021
@TrueBrain TrueBrain deleted the invalid-vehicle-orders-2 branch January 8, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LordAro LordAro LordAro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.

Impossible/invalid orders aren't marked as such in the order list
2 participants
@TrueBrain @LordAro