something is double into the track list if you build

[Realsteel89]

Operating System

Windows 10 64 bit

OpenRCT2 build

latest develop build

Base game

RollerCoaster Tycoon 2

Area(s) with this issue?

This is a development issue

Describe the issue

delete one into the code list

Steps to reproduce

the large half loop left is double into the track list

Attachments

[zzril]

Not sure on which commit and coaster the OP got this, but I managed to recreate this on currently latest develop (ed875a1) for the Twister Roller Coaster (standard trains) and Inverted Coaster. No plugins or cheats enabled.

It seems that for the left large half loop, the game distinguishes between those that make up the first half of a left-going loop and those that make up the second half of a right-going loop.
For the right large half loop piece, the game does not make this distinction, as for all the other inversions.

---

[zzril]

Enum definitions:

In src/openrct2/ride/Track.h:

namespace TrackElemType
{
// [...]
    constexpr track_type_t LeftCorkscrewUp = 58;
    constexpr track_type_t RightCorkscrewUp = 59;
    constexpr track_type_t LeftCorkscrewDown = 60;
    constexpr track_type_t RightCorkscrewDown = 61;
// [...]
    constexpr track_type_t LeftLargeHalfLoopUp = 183;
    constexpr track_type_t RightLargeHalfLoopUp = 184;
    constexpr track_type_t RightLargeHalfLoopDown = 185;
    constexpr track_type_t LeftLargeHalfLoopDown = 186;
// [...]
    constexpr track_type_t LeftMediumHalfLoopUp = 271;
    constexpr track_type_t RightMediumHalfLoopUp = 272;
    constexpr track_type_t LeftMediumHalfLoopDown = 273;
    constexpr track_type_t RightMediumHalfLoopDown = 274;

The large half loops are off.

---

Definition of coordinates:

In src/openrct2/ride/TrackData.cpp:

static constexpr TrackCoordinates _trackCoordinates[] = {
// [...]
        { 0, 3, 0, 80, -32, -32 },  // ELEM_LEFT_CORKSCREW_UP
        { 0, 1, 0, 80, -32, 32 },   // ELEM_RIGHT_CORKSCREW_UP
        { 0, 3, 0, -80, -32, -32 }, // ELEM_LEFT_CORKSCREW_DOWN
        { 0, 1, 0, -80, -32, 32 },  // ELEM_RIGHT_CORKSCREW_DOWN
// [...]
        { 0, 2, 0, 280, -64, -32 }, // ELEM_LEFT_LARGE_HALF_LOOP_UP
        { 0, 2, 0, 280, -64, 32 },  // ELEM_RIGHT_LARGE_HALF_LOOP_UP
        { 0, 2, 0, -280, 64, -32 }, // ELEM_RIGHT_LARGE_HALF_LOOP_DOWN
        { 0, 2, 0, -280, 64, 32 },  // ELEM_LEFT_LARGE_HALF_LOOP_DOWN
// [...]
        { 0, 2, 0, 216, -32, -32 },  // TrackElemType::LeftMediumHalfLoopUp
        { 0, 2, 0, 216, -32, 32 },   // TrackElemType::RightMediumHalfLoopUp
        { 0, 2, 0, -216, 32, -32 },  // TrackElemType::RightMediumHalfLoopDown
        { 0, 2, 0, -216, 32, 32 },   // TrackElemType::LeftMediumHalfLoopDown

Corkscrews and large half loops match their order from the enum definition. Medium loops don't.

---

Strings:

In src/openrct2/ride/TrackData.cpp:

static constexpr const StringId RideConfigurationStringIds[] = {
// [...]
    STR_HALF_CORKSCREW_LEFT,           // 58
    STR_HALF_CORKSCREW_RIGHT,          // 59
    STR_HALF_CORKSCREW_LEFT,           // 60
    STR_HALF_CORKSCREW_RIGHT,          // 61
// [...]
    STR_LARGE_HALF_LOOP_LEFT,          // 183
    STR_LARGE_HALF_LOOP_RIGHT,         // 184
    STR_LARGE_HALF_LOOP_LEFT,          // 185
    STR_LARGE_HALF_LOOP_RIGHT,         // 186
// [...]
    STR_MEDIUM_HALF_LOOP_LEFT,         // TrackElemType::LeftMediumHalfLoopUp
    STR_MEDIUM_HALF_LOOP_RIGHT,        // TrackElemType::RightMediumHalfLoopUp
    STR_MEDIUM_HALF_LOOP_LEFT,         // TrackElemType::RightMediumHalfLoopDown
    STR_MEDIUM_HALF_LOOP_RIGHT,        // TrackElemType::LeftMediumHalfLoopDown

For both loops, the comments don't match the string names...

---

Order in the UI / dropdown menu:

In src/openrct2-ui/ride/Construction.h:

/**
 * Order of special track elements dropdown. Elements with the same name string must be sequential or they show up twice.
 */
constexpr std::array DropdownOrder = {
// [...]
    TrackElemType::LeftCorkscrewUp,
    TrackElemType::LeftCorkscrewDown,
    TrackElemType::RightCorkscrewUp,
    TrackElemType::RightCorkscrewDown,
// [...]
    TrackElemType::LeftLargeHalfLoopUp,
    TrackElemType::LeftLargeHalfLoopDown,
    TrackElemType::RightLargeHalfLoopUp,
    TrackElemType::RightLargeHalfLoopDown,
// [...]
// Medium-size loops, for reference:
    TrackElemType::LeftMediumHalfLoopUp,
    TrackElemType::LeftMediumHalfLoopDown,
    TrackElemType::RightMediumHalfLoopUp,
    TrackElemType::RightMediumHalfLoopDown,

The comment refers to this code in src/openrct2-ui/ride/Construction.cpp:

SpecialElementsDropdownState BuildSpecialElementsList(
    const Ride& currentRide, uint8_t buildDirection, uint8_t buildSlope, uint8_t buildBank, RideConstructionState state)
{
// [...]
    for (track_type_t trackType : DropdownOrder)
    {
// [...]
        // Check if a previous element exists, to collate entries if possible
        if (!list.Elements.empty() && GetTrackElementDescriptor(list.Elements.back().TrackType).Description == ted.Description)
        {
// [...]
            // If the previous element is disabled and current element is enabled, replace the previous element
            if (lastElement.Disabled && !entryIsDisabled)
            {
                lastElement.TrackType = trackType;
                lastElement.Disabled = false;
                list.HasActiveElements = true;
                continue;
            }

[zzril]

Changing the order in src/openrct2-ui/ride/Construction.h to this seems to work, although I don't understand why:

TrackElemType::LeftLargeHalfLoopUp,
TrackElemType::RightLargeHalfLoopDown,
TrackElemType::RightLargeHalfLoopUp,
TrackElemType::LeftLargeHalfLoopDown,

[zzril]

The DropdownOrder array was introduced with 61d820e on develop (between release v0.4.4 and v0.4.5) and with it the bug.

@spacek531 If you're still around, can you explain what went wrong here?

[zzril]

Alright, I think I got it.

The naming of the TrackElemType constants in src/openrct2/ride/Track.h is wrong:

namespace TrackElemType
{
// [...]
    constexpr track_type_t LeftCorkscrewUp = 58;
    constexpr track_type_t RightCorkscrewUp = 59;
    constexpr track_type_t LeftCorkscrewDown = 60;
    constexpr track_type_t RightCorkscrewDown = 61;
// [...]
    constexpr track_type_t LeftLargeHalfLoopUp = 183;
    constexpr track_type_t RightLargeHalfLoopUp = 184;
    constexpr track_type_t RightLargeHalfLoopDown = 185;
    constexpr track_type_t LeftLargeHalfLoopDown = 186;
// [...]
    constexpr track_type_t LeftMediumHalfLoopUp = 271;
    constexpr track_type_t RightMediumHalfLoopUp = 272;
    constexpr track_type_t LeftMediumHalfLoopDown = 273;
    constexpr track_type_t RightMediumHalfLoopDown = 274;

What is defined above as the RightLargeHalfLoopDown is in fact a left large half loop down. (Which is, confusingly, the piece that completes a loop that was started with a RightLargeHalfLoopUp. But that's a naming convention that has been consistent throughout the game for inverting pieces that also come with a direction change (i.e. loops and corkscrews, but not inline twists).)

Code that does not refer to these elements by name, but just to the constant directly (either explicitely or implicitely, by using the enum values as array indices) - such as the _trackCoordinates array cited above - works as expected. The end user doesn't see the incorrect variable name that was used in the source code (unless checking in a debugger).

Code that refers to the elements by name must stick to the incorrect naming.

The above cited solution

TrackElemType::LeftLargeHalfLoopUp,
TrackElemType::RightLargeHalfLoopDown,
TrackElemType::RightLargeHalfLoopUp,
TrackElemType::LeftLargeHalfLoopDown,

in fact boils down to:

183,
185,
184,
186

which is the same numeric order as for the Medium loops:

    TrackElemType::LeftMediumHalfLoopUp,
    TrackElemType::LeftMediumHalfLoopDown,
    TrackElemType::RightMediumHalfLoopUp,
    TrackElemType::RightMediumHalfLoopDown,

or - in numbers:

271,
273,
272,
274

So, it makes sense that that solution produces the correct behaviour.

(And the reason why the incorrect order in the dropdown array makes the left large half loop show up twice is exactly what is explained in the comment there - two elements sharing the same name string must occur right after another in this array in order for the logic to detect and collapse them to one.)

[spacek531]

Nice sleuthing. I had forgotten about that quirk of naming.