Add ride ratings test #5047

IntelOrca · 2017-01-11T17:16:53Z

So we know if we have changed the calculation logic unintentionally.

marijnvdwerf · 2017-01-11T23:36:04Z

test/tests/RideRatings.cpp

+class RideRatings : public testing::Test
+{
+protected:
+    static const char * ExpectedRideRatings[];


I assume you are suggesting I make it utf8? There is nothing other than ascii in this file.

marijnvdwerf · 2017-01-11T23:36:09Z

test/tests/RideRatings.cpp

+
+    void CalculateRatingsForAllRides()
+    {
+        for (int rideId = 0; rideId < MAX_RIDES; rideId++)


marijnvdwerf · 2017-01-11T23:45:40Z

I'm not sure what' the ultimate idea with regard to the (s|u)int(8|16|32) types. There's #4936, but pretty much all new code seems to use normal ints etc.

IntelOrca · 2017-01-11T23:49:00Z

I should be using sint32, int in for loops is just a habbit.

Gymnasiast · 2017-05-15T12:33:23Z

What is the status of this?

IntelOrca · 2017-05-15T13:14:39Z

It needs an SV6 that has a reasonable amount of content in it, e.g. water, scenery the whole works with an instance of every single ride type in the park. Perhaps we can ask for one to be created on the forums?

janisozaur · 2017-06-04T19:58:36Z

src/openrct2/common.h

+#define UNUSED(x)  ((void)(x))
+
+// Macro for turning symbol names into strings
+#define SZ(x)       #x


What's wrong with nameof?

Nothing, just didn't know we had it.

janisozaur · 2017-06-04T20:07:05Z

src/openrct2/core/Path.hpp

@@ -23,6 +23,7 @@ namespace Path
 {
    utf8 * Append(utf8 * buffer, size_t bufferSize, const utf8 * src);
    std::string Combine(const std::string &a, const std::string &b);
+    std::string Combine(const std::string &a, const std::string &b, const std::string &c);


Why do we need two Combines? Isn't it synonymous with Combine(a, Combine(b, c))?

Why would you want to write Combine(a, Combine(b, c)) over Combine(a, b, c)?

For the same reason you would only have int add(int a, int b) and no int add(int a, int b, int c), int add(int a, int b, int c, int d) and so on…

I don't agree - the code clarity is better with an overload that takes three parameters. I would use vargs if possible.

Why not use variadic templates then? I'd rather see that than explicit implementation for n arguments.

I don't know how they work. Can you provide an example or online example?

https://wandbox.org/permlink/Go1VaEWuAKJuzJuN

janisozaur · 2017-06-04T20:11:07Z

src/openrct2/core/File.h

@@ -27,4 +28,5 @@ namespace File
    bool Move(const std::string &srcPath, const std::string &dstPath);
    void * ReadAllBytes(const std::string &path, size_t * length);
    void WriteAllBytes(const std::string &path, const void * buffer, size_t length);
+    std::vector<std::string> ReadAllLines(const std::string &path);


Why is it File.h and not File.hpp? I recall you provided a reason at some point, but I don't remember it.

hpp contains implementation code.

janisozaur · 2017-06-04T20:12:41Z

test/tests/RideRatings.cpp

+}
+
+using namespace OpenRCT2;
+using namespace OpenRCT2::Audio;


Was originally needed, I can remove it now.

IntelOrca · 2017-06-05T20:54:40Z

test/tests/CMakeLists.txt

+if (NOT "z$ENV{CI}" STREQUAL "z")
+    message("Detected CI environment. Disabling ride rating test.")
+else ()
+    execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink "${CMAKE_CURRENT_LIST_DIR}/testdata/parks" "${CMAKE_CURRENT_BINARY_DIR}/parks")


Can this be done at the top? Other tests may need testdata in the future.

This fails because you are symlinking parks rather than testdata.

Yeah, but it still fails because I forgot I run all these tests in docker which has no CI env variable set…

- Feature: [#1399 (partial), #5177] Add window that displays any missing/corrupt objects when loading a park - Feature: [#5056] Add cheat to own all land. - Feature: [#5133] Add option to display guest expenditure (as seen in RCTC). - Feature: [#5196] Add cheat to disable ride ageing. - Feature: [#5504] Group vehicles into ride groups - Feature: [#5576] Add a persistent 'display real names of guests' setting. - Feature: [#5611] Add support for Android - Feature: [#5706] Add support for OpenBSD - Feature: OpenRCT2 now starts up on the display it was last shown on. - Feature: Park entrance fee can now be set to amounts up to £200. - Improved: Construction rights can now be placed on park entrances. - Improved: Mouse can now be dragged to select scenery when saving track designs - Fix: [#259] Money making glitch involving swamps (original bug) - Fix: [#441] Construction rights over entrance path erased (original bug) - Fix: [#578] Ride ghosts show up in ride list during construction (original bug) - Fix: [#597] 'Finish 5 roller coasters' goal not properly checked (original bug) - Fix: [#667] Incorrect banner limit calculation (original bug) - Fix: [#739] Crocodile Ride (Log Flume) never allows more than five boats (original bug) - Fix: [#837] Can't move windows on title screen to where the toolbar would be (original bug) - Fix: [#1705] Time Twister's Medieval entrance has incorrect scrolling (original bug) - Fix: [#3178, #5456] Paths with non-ASCII characters not handled properly on macOS. - Fix: [#3346] Crash when extra long train breaks down at the back - Fix: [#3479] Building in pause mode creates too many floating numbers, crashing the game - Fix: [#3565] Multiplayer server crash - Fix: [#3681] Steel Twister rollercoaster always shows all track designs - Fix: [#3846, #5749] Crash when testing coaster with a diagonal lift in block brake mode - Fix: [#4054] Sorting rides by track type: Misleading research messages - Fix: [#4055] Sort rides by track type: Sorting rule is not really clear (inconsistent?) - Fix: [#4512] Invisible map edge tiles corrupted - Fix: [#5009] Ride rating calculations can overflow - Fix: [#5253] RCT1 park value conversion factor too high - Fix: [#5400] New Ride window does not focus properly on newly invented ride. - Fix: [#5489] Sprite index crash for car view on car ride. - Fix: [#5730] Unable to uncheck 'No money' in the Scenario Editor. - Fix: [#5750] Game freezes when ride queue linked list is corrupted. - Fix: [#5819] Vertical multi-dimension coaster tunnels drawn incorrectly - Fix: Non-invented vehicles can be used via track designs in select-by-track-type mode. - Fix: Track components added by OpenRCT2 are now usable in older scenarios. - Technical: [#5047] Add ride ratings tests - Technical: [#5458] Begin offering headless build with reduced compile- and run-time dependencies - Technical: [#5755] Title sequence wait periods use milliseconds - Technical: Fix many desync sources

marijnvdwerf reviewed Jan 11, 2017

View reviewed changes

IntelOrca force-pushed the develop branch from a5a633f to d9f9fcd Compare January 29, 2017 23:30

IntelOrca force-pushed the test/ride-ratings branch from b40b3e6 to 1a8a998 Compare May 17, 2017 17:36

IntelOrca force-pushed the test/ride-ratings branch 4 times, most recently from dfb4ca6 to fdce84a Compare June 3, 2017 17:05

IntelOrca added 12 commits June 4, 2017 00:47

Add ride ratings test

c5306d7

Force rating update on each ride before testing

443e17c

Fix test expectations and describe ride types

d65d31f

Change location of test data

cf7f1b7

Create test data helpers

b28fde6

Fix warning

7ca1173

Add File::ReadAllLines helper

f5a23d7

Use new rating test data

e9898ba

Add rating test data

f2524c0

Fix ride ratings test to work with new Context flow

0662f16

Enable ride ratings test for cmake

38fb37c

Upload test results to AppVeyor

79eef1d

IntelOrca force-pushed the test/ride-ratings branch from fdce84a to 79eef1d Compare June 3, 2017 23:49

janisozaur reviewed Jun 4, 2017

View reviewed changes

Clean up branch

9a619b3

IntelOrca and others added 3 commits June 5, 2017 17:46

Use varg template for Path::Combine

2d00b1e

Create symlink for rides test

46e9978

Disable ride ratings test on CI

c64076e

IntelOrca commented Jun 5, 2017

View reviewed changes

janisozaur added 3 commits June 5, 2017 23:08

Add option to force-disable RCT2 tests

eafd2ac

Fix symlink created for ride ratings test

105f353

Disable RCT2 tests on Travis

58c2faa

janisozaur approved these changes Jun 5, 2017

View reviewed changes

IntelOrca merged commit 58c2faa into OpenRCT2:develop Jun 5, 2017

janisozaur mentioned this pull request Jun 6, 2017

ride_ratings_test should be run on CIs #5552

Closed

IntelOrca deleted the test/ride-ratings branch March 16, 2018 21:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ride ratings test #5047

Add ride ratings test #5047

IntelOrca commented Jan 11, 2017

marijnvdwerf Jan 11, 2017

IntelOrca Jan 11, 2017

marijnvdwerf Jan 11, 2017

marijnvdwerf commented Jan 11, 2017

IntelOrca commented Jan 11, 2017 •

edited

Loading

Gymnasiast commented May 15, 2017

IntelOrca commented May 15, 2017

janisozaur Jun 4, 2017

IntelOrca Jun 4, 2017

janisozaur Jun 4, 2017

IntelOrca Jun 4, 2017

janisozaur Jun 5, 2017

IntelOrca Jun 5, 2017

janisozaur Jun 5, 2017

IntelOrca Jun 5, 2017

janisozaur Jun 5, 2017

IntelOrca Jun 5, 2017

janisozaur Jun 4, 2017

IntelOrca Jun 4, 2017

janisozaur Jun 4, 2017

IntelOrca Jun 4, 2017

IntelOrca Jun 5, 2017

IntelOrca Jun 5, 2017

janisozaur Jun 5, 2017

Add ride ratings test #5047

Add ride ratings test #5047

Conversation

IntelOrca commented Jan 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marijnvdwerf commented Jan 11, 2017

IntelOrca commented Jan 11, 2017 • edited Loading

Gymnasiast commented May 15, 2017

IntelOrca commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IntelOrca commented Jan 11, 2017 •

edited

Loading