Placing track designs at locations blocked by anything results in wrong error message

[ZehMatt]

Operating System

OpenRCT2 build

OpenRCT v0.3.4.1

Describe the issue

Instead of saying "Can't construct here, blocked by xyz" it always says "Land not owned".

Looks like it was introduced in 0.3.4(.1)

Area(s) with issue?

No response

Steps to reproduce

1. Pick any prebuilt coaster.
2. Place somewhere where it would normally say its blocked.

Attachments

[hjort96]

It used to say "Can't construct here, blocked by xyz"?
I've managed to make it not say "Land not owned by park!" when the land is owned by the park. The reason I'm asking is because I can't find an ID for a string that says "Can't construct here, blocked by xyz".

[Broxzier]

@hjort96 Have a look at map_obstruction_set_error_text. Here the strings are getting formatted.

OpenRCT2/src/openrct2/world/ConstructionClearance.cpp

Lines 279 to 344 in 30750b2

	void map_obstruction_set_error_text(TileElement* tileElement, GameActions::Result& res)
	{
	Ride* ride;
	res.ErrorMessage = STR_OBJECT_IN_THE_WAY;
	switch (tileElement->GetType())
	{
	case TILE_ELEMENT_TYPE_SURFACE:
	res.ErrorMessage = STR_RAISE_OR_LOWER_LAND_FIRST;
	break;
	case TILE_ELEMENT_TYPE_PATH:
	res.ErrorMessage = STR_FOOTPATH_IN_THE_WAY;
	break;
	case TILE_ELEMENT_TYPE_TRACK:
	ride = get_ride(tileElement->AsTrack()->GetRideIndex());
	if (ride != nullptr)
	{
	res.ErrorMessage = STR_X_IN_THE_WAY;
	Formatter ft(res.ErrorMessageArgs.data());
	ride->FormatNameTo(ft);
	}
	break;
	case TILE_ELEMENT_TYPE_SMALL_SCENERY:
	{
	auto* sceneryEntry = tileElement->AsSmallScenery()->GetEntry();
	res.ErrorMessage = STR_X_IN_THE_WAY;
	auto ft = Formatter(res.ErrorMessageArgs.data());
	rct_string_id stringId = sceneryEntry != nullptr ? sceneryEntry->name : static_cast<rct_string_id>(STR_EMPTY);
	ft.Add<rct_string_id>(stringId);
	break;
	}
	case TILE_ELEMENT_TYPE_ENTRANCE:
	switch (tileElement->AsEntrance()->GetEntranceType())
	{
	case ENTRANCE_TYPE_RIDE_ENTRANCE:
	res.ErrorMessage = STR_RIDE_ENTRANCE_IN_THE_WAY;
	break;
	case ENTRANCE_TYPE_RIDE_EXIT:
	res.ErrorMessage = STR_RIDE_EXIT_IN_THE_WAY;
	break;
	case ENTRANCE_TYPE_PARK_ENTRANCE:
	res.ErrorMessage = STR_PARK_ENTRANCE_IN_THE_WAY;
	break;
	}
	break;
	case TILE_ELEMENT_TYPE_WALL:
	{
	auto* wallEntry = tileElement->AsWall()->GetEntry();
	res.ErrorMessage = STR_X_IN_THE_WAY;
	auto ft = Formatter(res.ErrorMessageArgs.data());
	rct_string_id stringId = wallEntry != nullptr ? wallEntry->name : static_cast<rct_string_id>(STR_EMPTY);
	ft.Add<rct_string_id>(stringId);
	break;
	}
	case TILE_ELEMENT_TYPE_LARGE_SCENERY:
	{
	auto* sceneryEntry = tileElement->AsLargeScenery()->GetEntry();
	res.ErrorMessage = STR_X_IN_THE_WAY;
	auto ft = Formatter(res.ErrorMessageArgs.data());
	rct_string_id stringId = sceneryEntry != nullptr ? sceneryEntry->name : static_cast<rct_string_id>(STR_EMPTY);
	ft.Add<rct_string_id>(stringId);
	break;
	}
	}
	}

[duncanspumpkin]

Can't construct here, is an error title. The rest of it is an error message. So the message is getting a bit messed up. This happens quite easily due to all the globals involved

[ZehMatt]

One of the reasons this is happening is becaue of this:

OpenRCT2/src/openrct2/world/Map.cpp

Line 790 in 86f9158

gGameCommandErrorText = STR_LAND_NOT_OWNED_BY_PARK;

Basically anyone who calls map_is_location_owned overwrites the last set error text.

[hjort96]

Thanks @Broxzier!

Seems like after #15815 was merged no error message was displayed, just the title. However, assigning

gGameCommandErrorText = std::get<1>(res->ErrorMessage);

after res is assigned in this part of TrackDesign.cpp

OpenRCT2/src/openrct2/ride/TrackDesign.cpp

Lines 1685 to 1700 in 0e892e9

	gGameCommandErrorTitle = STR_RIDE_CONSTRUCTION_CANT_CONSTRUCT_THIS_HERE;
	auto trackPlaceAction = TrackPlaceAction(
	_currentRideIndex, trackType, { newCoords, tempZ, static_cast<uint8_t>(rotation) }, brakeSpeed, trackColour,
	seatRotation, liftHillAndAlternativeState, true);
	trackPlaceAction.SetFlags(flags);
	auto res = flags & GAME_COMMAND_FLAG_APPLY ? GameActions::ExecuteNested(&trackPlaceAction)
	: GameActions::QueryNested(&trackPlaceAction);
	money32 cost = res->Error == GameActions::Status::Ok ? res->Cost : MONEY32_UNDEFINED;
	totalCost += cost;
	if (cost == MONEY32_UNDEFINED)
	{
	return std::nullopt;
	}
	break;

seems to give (some) correct error messages.
I don't think this is an incorrect overwriting of gGameCommandErrorText because this seems to be last place we can determine what the error was before displaying the message, since after returning from this function the only information we have is that cost == MONEY32_UNDEFINED.

[duncanspumpkin]

@ZehMatt is doing a large refactor to track designs to fix the issue once and for all (and redesign some of the bad design). Just hold tight.