{"action":"created","issue":{"url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/6871","repository_url":"https://api.github.com/repos/OpenRCT2/OpenRCT2","labels_url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/6871/labels{/name}","comments_url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/6871/comments","events_url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/6871/events","html_url":"https://github.com/OpenRCT2/OpenRCT2/issues/6871","id":284681224,"number":6871,"title":"Ride test crash prevention doesn't prevent crashes if ride status is closed (\"single closed\") at the moment the crash actually occurs","user":{"login":"jgottula","id":822207,"avatar_url":"https://avatars1.githubusercontent.com/u/822207?v=4","gravatar_id":"","url":"https://api.github.com/users/jgottula","html_url":"https://github.com/jgottula","followers_url":"https://api.github.com/users/jgottula/followers","following_url":"https://api.github.com/users/jgottula/following{/other_user}","gists_url":"https://api.github.com/users/jgottula/gists{/gist_id}","starred_url":"https://api.github.com/users/jgottula/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jgottula/subscriptions","organizations_url":"https://api.github.com/users/jgottula/orgs","repos_url":"https://api.github.com/users/jgottula/repos","events_url":"https://api.github.com/users/jgottula/events{/privacy}","received_events_url":"https://api.github.com/users/jgottula/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2017-12-27T09:10:28Z","updated_at":"2017-12-29T20:23:45Z","closed_at":null,"author_association":"NONE","body":"**OS:** Windows 8.1\r\n**Version:** 0.1.2\r\n**Commit/Build:** 8fdec42 (x86-64 build)\r\n\r\n**Happens in vanilla RCT2:** Not applicable.\r\n**Happens in multiplayer:** Yep, both singleplayer and multiplayer.\r\n\r\n### Explanation\r\n\r\nSo, OpenRCT2 has this handy-dandy `no_test_crashes` feature that prevents rides that are undergoing testing from being able to crash. (Well... to be precise, it prevents \"fall off the track\" style crashes, not \"vehicle-on-vehicle collision\" style crashes; as per #5688.)\r\n\r\nOut of habit (and in part to prevent potential vehicle-on-vehicle collisions during testing), on my coasters I'll usually hit the yellow \"test\" button once, allow a single train to leave the station, and then hit the red \"close\" button once, such that the ride is \"single-closed\" (it won't send more trains out) but not \"double-closed\" (it's not *completely* reset).\r\n\r\nI found that this consistently leads to the test crash prevention feature not working: trains will fly off the end of the track and crash if the layout permits it, without the game instantly resetting the ride at the last moment as expected.\r\n\r\n### Video Demonstration\r\n\r\n**[Here's a quick video demo I put together.](https://youtu.be/M9Fu6MmdKFw)**\r\n\r\nFor the first two test runs, I leave the ride in testing mode, and the crash prevention reset mechanism (effectively: \"**close**; **close**; **test**\") works fine.\r\n\r\nFor the next two runs, I switch the ride from testing to closed mode after the first train leaves the station. Consequently, the crash prevention feature doesn't kick in as the train flies off the track.\r\n\r\nFor the last two runs, I leave the ride in testing mode, just as in the first two runs.\r\n\r\n### Code Analysis\r\n\r\nUpon looking through the source code, it became fairly obvious what's going on here: the test crash prevention code checks the ride status (open/testing/closed) at the moment of the potential crash, and only prevents crashes if the *current ride status at the moment that the vehicle is crashing* is `RIDE_STATUS_TESTING`.\r\nhttps://github.com/OpenRCT2/OpenRCT2/blob/8fdec42ff9b8956f6c502b683dec08141ec20fc7/src/openrct2/ride/ride.c#L2044\r\n\r\nSo obviously that's a bit overly simplistic. But having dug around the source code some more, I can see that the ride status field really is fairly useless for this purpose, given the dynamic manner in which players can switch rides between open/testing/closed at will, with various vehicles in motion on the track at any given moment that could have been from either the \"open\" or \"testing\" modes of operation.\r\n\r\nAnd I'm not sure there really is a robust way to determine whether a particular ride vehicle was *originally* sent out of the station for testing purposes specifically. (I mean, yeah, sure, there's `VEHICLE_UPDATE_FLAG_TESTING`; but that can get invalidated by all kinds of stuff via `invalidate_test_results`.)\r\n\r\n----\r\n\r\nI get the strong sense that this is gonna end up being one of those \"we'd need our own extended save format to add new bits so we could reliably track whether each vehicle was sent out for testing / what the ride's status was back at the time the vehicle was sent out\" sort of things; and quite likely a healthy dose of \"this feature is just a kludge anyway, which we really want to replace with a proper 'ghost train' thing at some point anyway\" as well. Which I understand.\r\n\r\nBut I figured this was worth reporting, **(a)** so that it's at least a documented known issue; **(b)** to maybe increase motivation toward implementing proper \"ghost trains\" for ride testing; and **(c)** in case there *does* happen to be a currently-feasible, non-horrendous way of addressing this issue, that perhaps it could be implemented. :wink:"},"comment":{"url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/comments/354496218","html_url":"https://github.com/OpenRCT2/OpenRCT2/issues/6871#issuecomment-354496218","issue_url":"https://api.github.com/repos/OpenRCT2/OpenRCT2/issues/6871","id":354496218,"user":{"login":"Gymnasiast","id":1478678,"avatar_url":"https://avatars2.githubusercontent.com/u/1478678?v=4","gravatar_id":"","url":"https://api.github.com/users/Gymnasiast","html_url":"https://github.com/Gymnasiast","followers_url":"https://api.github.com/users/Gymnasiast/followers","following_url":"https://api.github.com/users/Gymnasiast/following{/other_user}","gists_url":"https://api.github.com/users/Gymnasiast/gists{/gist_id}","starred_url":"https://api.github.com/users/Gymnasiast/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Gymnasiast/subscriptions","organizations_url":"https://api.github.com/users/Gymnasiast/orgs","repos_url":"https://api.github.com/users/Gymnasiast/repos","events_url":"https://api.github.com/users/Gymnasiast/events{/privacy}","received_events_url":"https://api.github.com/users/Gymnasiast/received_events","type":"User","site_admin":false},"created_at":"2017-12-29T20:23:44Z","updated_at":"2017-12-29T20:23:44Z","author_association":"MEMBER","body":"> Okay. Had kinda been hoping that there might be a way to kludge it with existing state information\r\n\r\nExactly: kludge into. But we have only done so in an extremely limited number of occasions, as it makes importing old saves harder (especially now RCTC has also started kludging stuff into SV6). Some stuff is possible to hack into SV6, but not always a good idea. In any case: yes, this feature will be replaced with a ghost train.\r\n\r\nI can't really speak for Duncan, but sometimes we have different opinions about the best course of action. After all, this is a loosely-organised project."}}
{"actor":{"display_login":"Gymnasiast"}}