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

Display colliding vehicle name in mapgen failure messages #76659

Merged
merged 5 commits into from
Sep 29, 2024

Conversation

PatrikLundell
Copy link
Contributor

@PatrikLundell PatrikLundell commented Sep 25, 2024

Summary

None

Purpose of change

Provide some help in trying to find out why mapgen (and base camp upgrades in particular) fail by providing the name of the first colliding "vehicle" encountered in the error message.

This was prompted by #76627, the save of which was used for testing.

Describe the solution

Change a whole lot of boolean functions to return strings instead, where an empty string corresponds to true/success, and the string otherwise containing the name of the offending vehicle/appliance.
There are a couple of error cases where the cause isn't actually a vehicle/appliance, so the string for those cases make that clear (which results in a weird looking message should these cases actually occur.

Describe alternatives you've considered

Testing

Loaded the save from #76627 (second character) and tried to finish the expansion, resulting in the following error message:
image

Additional context

It seems the vehicle display name operation prepends a "the" to the name, so I remove the "the" that was in the strings previously.
While looking for "the the" I fixed a number of comments as a by-catch.

Note that the error report only reports on the first clashing vehicle/appliance, so you might have to make a fair number of passes to find them all, but it's better than no indications at all.

@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display [C++] Changes (can be) made in C++. Previously named `Code` Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Player Faction Base / Camp All about the player faction base/camp/site astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Sep 25, 2024
Copy link
Member

@RenechCDDA RenechCDDA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change a whole lot of boolean functions to return strings instead, where an empty string corresponds to true/success, and the string otherwise containing the name of the offending vehicle/appliance.

I deeply recommend using ret_val instead, similar to #65341. Checking whether the string is empty is not good, among other problems the vehicle's name might be an empty string.

@PatrikLundell PatrikLundell marked this pull request as draft September 25, 2024 16:32
@PatrikLundell
Copy link
Contributor Author

I'm going to try to understand the ret_val stuff and shift to using it. Thus, this PR is set to draft.
Thanks for the recommendation.

@PatrikLundell PatrikLundell marked this pull request as ready for review September 26, 2024 10:51
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 26, 2024
@Maleclypse
Copy link
Member

@PatrikLundell I probably should have left conflict resolution to you. I was trying to match what I thought we were moving towards for tripoints but now it looks like I removed a file you were changing. ranged.cpp. My apologies if that is the case.

@PatrikLundell
Copy link
Contributor Author

If something was lost we'll eventually do it again (and potentially have to do it sooner, if it results in failures to compile). No big deal in either case, but it's nice to be notified, rather than end up with a sense of déja vue later.

@Maleclypse Maleclypse merged commit c199b93 into CleverRaven:master Sep 29, 2024
26 checks passed
@PatrikLundell PatrikLundell deleted the report_vehicle_collision branch September 29, 2024 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Player Faction Base / Camp All about the player faction base/camp/site
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants