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

Accident/disaster news not always showing #7611

Closed
lukasradek opened this issue May 29, 2019 · 18 comments · Fixed by #8497
Closed

Accident/disaster news not always showing #7611

lukasradek opened this issue May 29, 2019 · 18 comments · Fixed by #8497
Labels
bug
Milestone

Comments

@lukasradek
Copy link

@lukasradek lukasradek commented May 29, 2019

Version of OpenTTD

1.9.1

Expected result

When airplane crashes, disaster happens, or car is destroyed at the railroad crossing, news should be (edit: always) issued to the player.

Actual result

No news show up (in settings set to show full news).
(edit: News might be ommited espetially later in the game, where there is a lot of other news and advisors. It might leed to unknowingly loosing vehicles)

Steps to reproduce

Build airport with short runway, let airplane repeatedly land there, wait. When airplane eventually crashes, no report is shown.
Same with vehicles rammed by trains, or disasters destroying infrastructure.

Solution

Never ommit accident/crash/disaster related news.

@glx22
Copy link
Contributor

@glx22 glx22 commented May 29, 2019

I just tried road vehicle/train collision and I get the accident news as expected.

@Eddi-z
Copy link
Contributor

@Eddi-z Eddi-z commented May 29, 2019

News may occasionally be omitted if there are lots of ticker messages scrolling by

@lukasradek
Copy link
Author

@lukasradek lukasradek commented May 29, 2019

News may occasionally be omitted if there are lots of ticker messages scrolling by

That seems like the issue. Changing the OP to "don't ommit those reports" since in later game, once you news feed is busy, you basically after a while end up surprised, that you have no aircraft/cars left, because you missed those messages.

@lukasradek lukasradek changed the title Accident/disaster news not showing Accident/disaster news not always showing May 29, 2019
@EgyLynx
Copy link

@EgyLynx EgyLynx commented Jun 24, 2019

If there at something else get news it get these first.. Game not look these priority... tell that one loco wait deposit when it already changed new and go on back bussines??

@James103
Copy link
Contributor

@James103 James103 commented Aug 2, 2019

Affects 20190723-master-g2e686ad5d5, also in that when the game is in fast forward, most (sometimes all) summary messages are not shown. This not only includes accidents/disasters but also company info, general info, and other messages marked to display as "Summary".

@giordy
Copy link

@giordy giordy commented Mar 30, 2020

I confirm this. Even in multiplayer games, it happens that the opponents get the notification of some accident involving my vehicles far before that I get it, but it even happens that I don't get it at all.
And of course getting timely notifications about accidents is crucial to be able to replace the destroyed vehicle(s).

@giordy
Copy link

@giordy giordy commented Apr 27, 2020

I also noticed in a multiplayer game that I was getting all the notifications for plane crashes (I didn't own any) and my friends were getting none. My company was the first in the list, in case this info is useful.
We were playing on a very competitive map (infrastructure costs, inflation, very expensive airplanes) so there were "really" few planes, like 5-10 in total given the costs to maintain them. The volume of notifications was relatively low hence I suspect that there is some bug.

@embeddedt
Copy link
Contributor

@embeddedt embeddedt commented Apr 27, 2020

I've noticed something similar when playing singleplayer games with AIs (albeit with JGRPP, although I don't know if the difference matters here). I get a very high number of notifications about plane crashes; it's actually quite difficult to see any other notifications as a result.

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

@TrueBrain TrueBrain commented Jan 5, 2021

Did a bit of digging .. news is, as far as I can test and debug, working as intended. What you might be noticing, is this:

  • News that has a popup cannot be interrupted by other news that shows a popup; in other words, there is no priority in news, so if there is something else that has a popup, it will not show the crashed plane
  • If the crashed plane is cleaned up before the news is shown, you never get the see the news. This is because if the Vehicle is cleaned up, so are any news messages associated with the vehicle. This is including the news history!

The first in combination with the second can make it likely you don't see crashes. Cleanup happens relatively quick after crash, but news popups stay on for a very long time (relative). So if any other message pops up, it is very likely you are going to miss the crash.

The second alone is very annoying, as your history will just remove the crashed event as soon as the vehicle is cleaned up. Making it even more likely for you to miss this crucial event.

The problem is worse with FastForward btw, as news popups are shown for N seconds, not N game-days.

I am not sure what a proper solution is .. we remove the news, as the "viewport" in the news message is no longer valid, so that is a bit of a problem. We could extend how long a crashed vehicle is alive, but that also hinders other traffic to such airport. I have to think a bit about this.

@embeddedt : what you experience, really sounds like a you problem :D If you have enough planes to crash that it is constantly in the news .. sounds like something else is going wrong :) Either way, this is not part of this ticket, and I think even unlikely to be solved honestly :) If you want us to look into it, please make a new ticket for it.

@lukasradek
Copy link
Author

@lukasradek lukasradek commented Jan 5, 2021

@TrueBrain Full disclosure, I am not familiar with how OpenTTD code is designed, but I am a programmer so let me suggest a few things.

Would it be possible to
a) separate messages from their initiating vehicles? This would seem a better design to me anyways. But of course, you wouldn't be able to inspect the vehicle if you see the message to late. Any possibility to keep crashed vehicles in some "graveyard depo", so that the data can be accessed and the vehicle would not occupy the road? The user can then delete them as usual. The news viewport could be than tied to a particular location, where the crash happened and have a "button" to view the crashed vehicle in the "graveyard depo".

b) prioritize crash messages in the popup queue?
c) create separate / assign different place to show crash messages? Probably not a good idea 😁.

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 5, 2021

I just created a PR that indeed shows the news of the location the vehicle crashed; this now survives removing of vehicles. So there is that :)

The rest of what you suggest at a) we did discuss, and agree it would be better. But that is outside this issue, and a good feature request for anyone to implement. So if you feel up for it, and want to see how OpenTTD is coded, contributions are very welcome :D

b) is possible, but I am not sure if it is any solution. People could argue which news is more important .. I think with my suggested fix, it is already easier to spot crashed vehicles :)

c) is indeed properly not a good idea; the GUI is already bloated as it is :D

Tnx for thinking along, it seems that showing the news of the location it is crashed is the best solution for now :)

@lukasradek
Copy link
Author

@lukasradek lukasradek commented Jan 5, 2021

Might accually be a nice undertaking and contribution for me to do. Hope OpenTTD didn't decide to honor the Assembly language choice, that Chris did back in the day. 😁

So some kind of graveyard depo (not in the map if possible), which would accept crashed vehicles is within the capabilities of the design?

In your current fix, do you somehow solve for the fact, that user would get a crash news, but will not be able to found out which vehicle that was? Or more relevantly, reproduce its schedule?

@James103
Copy link
Contributor

@James103 James103 commented Jan 5, 2021

Hope OpenTTD didn't decide to honor the Assembly language choice, that Chris did back in the day.

OpenTTD is largely written in C++, with some Squirrel mixed in for AI/GS support.

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 5, 2021

In your current fix, do you somehow solve for the fact, that user would get a crash news, but will not be able to found out which vehicle that was? Or more relevantly, reproduce its schedule?

Indeed; if you are too late to spot it, you won't know what vehicle crashed. This was already the case before my fix, and will be after :)

So some kind of graveyard depo (not in the map if possible), which would accept crashed vehicles is within the capabilities of the design?

Doesn't have to be a real depot, but you can hide a vehicle completely, while keeping it around. Details I have not thought out, tbh, but I guess hiding the vehicle completely, and allowing some way to list crashed vehicles and "rebuild at nearest depot" or something like that. It seems #8498 already discusses this a bit :)

@lukasradek
Copy link
Author

@lukasradek lukasradek commented Jan 5, 2021

Sounds good 🙂. I will try to get some free time on my hand and might get to it. Thanks for the chat and quickfix and all the best in 2021.

@JGRennison
Copy link
Contributor

@JGRennison JGRennison commented Jan 5, 2021

Having vehicle instances which are "off-map" such that Vehicle::tile does not necessarily point to a valid sensible tile for that vehicle type requires changing a surprisingly large number of places in the code.

e.g. if a road vehicle was destroyed on a level crossing, later put into a "hidden" state, and then the crossing was removed to prevent further carnage, many parts of the code would encounter issues trying to get the road/tram type of the road vehicle's current tile, even if the vehicle was crashed and not moving.

This is certainly doable (it is done for trains in my branch), but it requires a non-trivial amount of combing through the code.

@Eddi-z
Copy link
Contributor

@Eddi-z Eddi-z commented Jan 5, 2021

my train of thought would be to teleport the vehicle into the nearest appropriate depot (hoping there is one).
that would avoid all the stuff about querying the tile the vehicle is on.

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jan 5, 2021

How about we continue this conversation in #8498, to not get away too much from the bug here, and possible features we like to have? Might be good for future-us! :)

TrueBrain added a commit that referenced this issue Jan 5, 2021
… up (#8497)

When a vehicle is cleaned up, all news that points to the news is
also removed. This was a bit evil, as it would also remove any
news related to crashed, acting like the crash never happened.
This left players a bit in the dark what was going on exactly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants