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

Converting town-owned road returns ambiguous/unhelpful error message #8162

Closed
James103 opened this issue May 19, 2020 · 3 comments
Closed

Converting town-owned road returns ambiguous/unhelpful error message #8162

James103 opened this issue May 19, 2020 · 3 comments

Comments

@James103
Copy link
Contributor

@James103 James103 commented May 19, 2020

Version of OpenTTD

master-20200509 (since NotRoadTypes was merged)

Expected result

The error message returned is something like "owned by {TOWN}" (STR_ERROR_OWNED_BY).

Actual result

The error message returned is "incompatible road" (STR_ERROR_INCOMPATIBLE_ROAD).

Steps to reproduce

  1. Load a roadtype NewGRF.
  2. Start a new game.
  3. Attempt to convert the road type to something else for a town-owned road.

Note

The following appear to be the only two unique occurences of the error message. They were both added in the NRT branch.

error.MakeError(STR_ERROR_INCOMPATIBLE_ROAD);

error.MakeError(STR_ERROR_INCOMPATIBLE_ROAD);

If both are removed, then the error message may also need to be removed as well as it would no longer be used.

@auge8472
Copy link
Contributor

@auge8472 auge8472 commented May 19, 2020

The error message occurs in my games (JGRPP 0.34.2) every time, I use the convert tool and the road piece already has the type, I want to convert it to (present road type == target type). It occurs whoever the road piece owns (town or me).

@TrevorShelton
Copy link
Contributor

@TrevorShelton TrevorShelton commented Jun 27, 2020

I think these two incompatibility messages may be in place instead of the owned by message because since an error has to be made artificially with MakeError rather than failing organically there is no information about the owner that is given to CommandCost, resulting in it making something up if there needs to be an owner and sometimes crashing the game in the process. That seems to be what happened when I did the substitutions. The incompatibility strings are the same each time and only require the fact that there was an error, not the nonexistent information besides that.

Perhaps arguments could be added to MakeError, but that is beyond the scope of this issue.

Honestly, I don't see the issue with just allowing road conversions within towns in the first place. These roads can already be manipulated like any other road except in this instance, the alteration comes out of your pocket in the same way it would for other roads, and while this would essentially make the base road already paid for you, the vehicles don't care about whether a road was paid for by you or not. The only thing that might need to be added is potentially an ownership change. And, of course, this would still make the incompatible road error never used.

EDIT: Given that you get relationship penalties for destroying roads in a town the same should happen for conversions since the road here is as well ruined for their purposes. And since it is already so the owner should switch to avoid more penalties for more changes if it doesn't already since you can't ruin it more. I'd have to look through it more to know what would need to be done regarding this.

@TrevorShelton
Copy link
Contributor

@TrevorShelton TrevorShelton commented Jun 28, 2020

Actually, scratch that. Simply adding

GetNameofOwner(OWNER_TOWN, tile);

under the two occurrences of incompatible road in addition to replacing them with owned by solves the owner name problem and crash.

LordAro added a commit that referenced this issue Jul 10, 2020
This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.
@LordAro LordAro closed this Jul 10, 2020
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jul 30, 2020
This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.
LordAro added a commit to LordAro/OpenTTD that referenced this issue Jul 30, 2020
This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.
LordAro added a commit that referenced this issue Aug 9, 2020
This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants